- Devel - libvirt List Archives

[libvirt] [PATCH] Quieten build & ensure API build scripts exit with non-zero status
by Daniel P. Berrange 19 Jul '11

19 Jul '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> The current API build scripts will continue and exit with a zero status even if they find problems. This has been the cause of many build problems, or hidden build errors, in the past. Change the scripts so they always exit with a non-zero status for any problems they do not understand. Also turn off all debug output by default so they respect $(AM_V_GEN) * docs/Makefile.am: Use $(AM_V_GEN) for API/HTML scripts * docs/apibuild.py, python/generator.py: Exit with non-zero status if problems are found. Also be silent, not outputting any debug messages. * src/Makefile.am: Use $(AM_V_GEN) for ESX generator * python/Makefile.am: Tweak rule --- docs/Makefile.am | 11 ++---- docs/apibuild.py | 95 ++++++++++++++++++++++++++++++-------------------- python/Makefile.am | 6 ++-- python/generator.py | 16 +++++--- src/Makefile.am | 2 +- 5 files changed, 75 insertions(+), 55 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 7ff94a0..4ba6b9d 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -153,21 +153,18 @@ internals/%.html.tmp: internals/%.html.in subsite.xsl page.xsl sitemap.html.in html/index.html: libvirt-api.xml newapi.xsl page.xsl sitemap.html.in - -@if [ -x $(XSLTPROC) ] ; then \ - echo "Rebuilding the HTML pages from the XML API" ; \ + $(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \ $(XSLTPROC) --nonet -o $(srcdir)/ \ - $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml ; fi - -@if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \ + $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml ; fi ; \ + if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \ if $(XMLCATALOG) '$(XML_CATALOG_FILE)' "-//W3C//DTD XHTML 1.0 Strict//EN" \ > /dev/null ; then \ - echo "Validating the resulting XHTML pages" ; \ SGML_CATALOG_FILES='$(XML_CATALOG_FILE)' \ $(XMLLINT) --catalogs --nonet --valid --noout $(srcdir)/html/*.html ; \ else echo "missing XHTML1 DTD" ; fi ; fi $(addprefix $(srcdir)/,$(devhelphtml)): $(srcdir)/libvirt-api.xml $(devhelpxsl) - -@echo Rebuilding devhelp files - -@if [ -x $(XSLTPROC) ] ; then \ + $(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \ $(XSLTPROC) --nonet -o $(srcdir)/devhelp/ \ $(top_srcdir)/docs/devhelp/devhelp.xsl $(srcdir)/libvirt-api.xml ; fi diff --git a/docs/apibuild.py b/docs/apibuild.py index f31a853..cb273dc 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -11,6 +11,8 @@ import os, sys import string import glob +quiet=1 +warnings=0 debug=0 debugsym=None @@ -95,7 +97,7 @@ class identifier: self.conditionals = None else: self.conditionals = conditionals[:] - if self.name == debugsym: + if self.name == debugsym and not quiet: print "=> define %s : %s" % (debugsym, (module, type, info, extra, conditionals)) @@ -155,7 +157,7 @@ class identifier: def update(self, header, module, type = None, info = None, extra=None, conditionals=None): - if self.name == debugsym: + if self.name == debugsym and not quiet: print "=> update %s : %s" % (debugsym, (module, type, info, extra, conditionals)) if header != None and self.header == None: @@ -203,7 +205,7 @@ class index: if d != None and name != None and type != None: self.references[name] = d - if name == debugsym: + if name == debugsym and not quiet: print "New ref: %s" % (d) return d @@ -244,9 +246,9 @@ class index: elif type == "macro": self.macros[name] = d else: - print "Unable to register type ", type + self.warning("Unable to register type ", type) - if name == debugsym: + if name == debugsym and not quiet: print "New symbol: %s" % (d) return d @@ -260,8 +262,8 @@ class index: if self.macros.has_key(id): del self.macros[id] if self.functions.has_key(id): - print "function %s from %s redeclared in %s" % ( - id, self.functions[id].header, idx.functions[id].header) + self.warning("function %s from %s redeclared in %s" % ( + id, self.functions[id].header, idx.functions[id].header)) else: self.functions[id] = idx.functions[id] self.identifiers[id] = idx.functions[id] @@ -273,15 +275,15 @@ class index: if self.macros.has_key(id): del self.macros[id] if self.variables.has_key(id): - print "variable %s from %s redeclared in %s" % ( - id, self.variables[id].header, idx.variables[id].header) + self.warning("variable %s from %s redeclared in %s" % ( + id, self.variables[id].header, idx.variables[id].header)) else: self.variables[id] = idx.variables[id] self.identifiers[id] = idx.variables[id] for id in idx.structs.keys(): if self.structs.has_key(id): - print "struct %s from %s redeclared in %s" % ( - id, self.structs[id].header, idx.structs[id].header) + self.warning("struct %s from %s redeclared in %s" % ( + id, self.structs[id].header, idx.structs[id].header)) else: self.structs[id] = idx.structs[id] self.identifiers[id] = idx.structs[id] @@ -294,8 +296,8 @@ class index: self.identifiers[id] = idx.unions[id] for id in idx.typedefs.keys(): if self.typedefs.has_key(id): - print "typedef %s from %s redeclared in %s" % ( - id, self.typedefs[id].header, idx.typedefs[id].header) + self.warning("typedef %s from %s redeclared in %s" % ( + id, self.typedefs[id].header, idx.typedefs[id].header)) else: self.typedefs[id] = idx.typedefs[id] self.identifiers[id] = idx.typedefs[id] @@ -311,15 +313,15 @@ class index: if self.enums.has_key(id): continue if self.macros.has_key(id): - print "macro %s from %s redeclared in %s" % ( - id, self.macros[id].header, idx.macros[id].header) + self.warning("macro %s from %s redeclared in %s" % ( + id, self.macros[id].header, idx.macros[id].header)) else: self.macros[id] = idx.macros[id] self.identifiers[id] = idx.macros[id] for id in idx.enums.keys(): if self.enums.has_key(id): - print "enum %s from %s redeclared in %s" % ( - id, self.enums[id].header, idx.enums[id].header) + self.warning("enum %s from %s redeclared in %s" % ( + id, self.enums[id].header, idx.enums[id].header)) else: self.enums[id] = idx.enums[id] self.identifiers[id] = idx.enums[id] @@ -330,10 +332,10 @@ class index: # check that function condition agrees with header if idx.functions[id].conditionals != \ self.functions[id].conditionals: - print "Header condition differs from Function for %s:" \ - % id - print " H: %s" % self.functions[id].conditionals - print " C: %s" % idx.functions[id].conditionals + self.warning("Header condition differs from Function for %s:" \ + % id) + self.warning(" H: %s" % self.functions[id].conditionals) + self.warning(" C: %s" % idx.functions[id].conditionals) up = idx.functions[id] self.functions[id].update(None, up.module, up.type, up.info, up.extra) # else: @@ -356,12 +358,13 @@ class index: def analyze(self): - self.analyze_dict("functions", self.functions) - self.analyze_dict("variables", self.variables) - self.analyze_dict("structs", self.structs) - self.analyze_dict("unions", self.unions) - self.analyze_dict("typedefs", self.typedefs) - self.analyze_dict("macros", self.macros) + if not quiet: + self.analyze_dict("functions", self.functions) + self.analyze_dict("variables", self.variables) + self.analyze_dict("structs", self.structs) + self.analyze_dict("unions", self.unions) + self.analyze_dict("typedefs", self.typedefs) + self.analyze_dict("macros", self.macros) class CLexer: """A lexer for the C language, tokenize the input by reading and @@ -621,6 +624,8 @@ class CParser: info, extra, self.conditionals) def warning(self, msg): + global warnings + warnings = warnings + 1 if self.no_error: return print msg @@ -1802,7 +1807,8 @@ class CParser: return token def parse(self): - self.warning("Parsing %s" % (self.filename)) + if not quiet: + print "Parsing %s" % (self.filename) token = self.token() while token != None: if token[0] == 'name': @@ -1869,7 +1875,8 @@ class docBuilder: pass def analyze(self): - print "Project %s : %d headers, %d modules" % (self.name, len(self.headers.keys()), len(self.modules.keys())) + if not quiet: + print "Project %s : %d headers, %d modules" % (self.name, len(self.headers.keys()), len(self.modules.keys())) self.idx.analyze() def scanHeaders(self): @@ -1995,7 +2002,7 @@ class docBuilder: else: output.write(" <field name='%s' type='%s' info='%s'/>\n" % (field[1] , field[0], desc)) except: - print "Failed to serialize struct %s" % (name) + self.warning("Failed to serialize struct %s" % (name)) output.write(" </struct>\n") else: output.write("/>\n"); @@ -2023,7 +2030,7 @@ class docBuilder: def serialize_function(self, output, name): id = self.idx.functions[name] - if name == debugsym: + if name == debugsym and not quiet: print "=>", id output.write(" <%s name='%s' file='%s' module='%s'>\n" % (id.type, @@ -2059,7 +2066,7 @@ class docBuilder: output.write(" <arg name='%s' type='%s' info='%s'/>\n" % (param[1], param[0], escape(param[2]))) self.indexString(name, param[2]) except: - print "Failed to save function %s info: " % name, `id.info` + self.warning("Failed to save function %s info: " % name, `id.info`) output.write(" </%s>\n" % (id.type)) def serialize_exports(self, output, file): @@ -2074,7 +2081,7 @@ class docBuilder: escape(dict.info[data]), string.lower(data))) except: - print "Header %s lacks a %s description" % (module, data) + self.warning("Header %s lacks a %s description" % (module, data)) if dict.info.has_key('Description'): desc = dict.info['Description'] if string.find(desc, "DEPRECATED") != -1: @@ -2287,7 +2294,8 @@ class docBuilder: def serialize(self): filename = "%s/%s-api.xml" % (self.path, self.name) - print "Saving XML description %s" % (filename) + if not quiet: + print "Saving XML description %s" % (filename) output = open(filename, "w") output.write('<?xml version="1.0" encoding="ISO-8859-1"?>\n') output.write("<api name='%s'>\n" % self.name) @@ -2323,7 +2331,8 @@ class docBuilder: output.close() filename = "%s/%s-refs.xml" % (self.path, self.name) - print "Saving XML Cross References %s" % (filename) + if not quiet: + print "Saving XML Cross References %s" % (filename) output = open(filename, "w") output.write('<?xml version="1.0" encoding="ISO-8859-1"?>\n') output.write("<apirefs name='%s'>\n" % self.name) @@ -2336,7 +2345,8 @@ def rebuild(): builder = None srcdir = os.environ["srcdir"] if glob.glob(srcdir + "/../src/libvirt.c") != [] : - print "Rebuilding API description for libvirt" + if not quiet: + print "Rebuilding API description for libvirt" dirs = [srcdir + "/../src", srcdir + "/../src/util", srcdir + "/../include/libvirt"] @@ -2344,12 +2354,13 @@ def rebuild(): dirs.append("../include/libvirt") builder = docBuilder("libvirt", srcdir, dirs, []) elif glob.glob("src/libvirt.c") != [] : - print "Rebuilding API description for libvirt" + if not quiet: + print "Rebuilding API description for libvirt" builder = docBuilder("libvirt", srcdir, ["src", "src/util", "include/libvirt"], []) else: - print "rebuild() failed, unable to guess the module" + self.warning("rebuild() failed, unable to guess the module") return None builder.scan() builder.analyze() @@ -2368,5 +2379,13 @@ if __name__ == "__main__": if len(sys.argv) > 1: debug = 1 parse(sys.argv[1]) + if warnings > 0: + sys.exit(2) + else: + sys.exit(0) else: rebuild() + if warnings > 0: + sys.exit(2) + else: + sys.exit(0) diff --git a/python/Makefile.am b/python/Makefile.am index 0edb3e4..a4c9a6b 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -53,11 +53,11 @@ GENERATED= libvirt-export.c \ libvirt.h \ libvirt.py -generated.stamp: $(srcdir)/$(GENERATE) $(API_DESC) - $(PYTHON) $(srcdir)/$(GENERATE) $(PYTHON) +$(GENERATE).stamp: $(srcdir)/$(GENERATE) $(API_DESC) + $(AM_V_GEN)$(PYTHON) $(srcdir)/$(GENERATE) $(PYTHON) && \ touch $@ -$(GENERATED): generated.stamp +$(GENERATED): $(GENERATE).stamp $(libvirtmod_la_OBJECTS): $(GENERATED) diff --git a/python/generator.py b/python/generator.py index c27ff73..d3a3616 100755 --- a/python/generator.py +++ b/python/generator.py @@ -11,6 +11,8 @@ import sys import string import re +quiet=1 + if __name__ == "__main__": # launched as a script srcPref = os.path.dirname(sys.argv[0]) @@ -617,7 +619,8 @@ def buildStubs(): sys.exit(1) n = len(functions.keys()) - print "Found %d functions in libvirt-api.xml" % (n) + if not quiet: + print "Found %d functions in libvirt-api.xml" % (n) py_types['pythonObject'] = ('O', "pythonObject", "pythonObject", "pythonObject") try: @@ -629,8 +632,8 @@ def buildStubs(): except IOError, msg: print file, ":", msg - - print "Found %d functions in libvirt-override-api.xml" % ( + if not quiet: + print "Found %d functions in libvirt-override-api.xml" % ( len(functions.keys()) - n) nb_wrap = 0 failed = 0 @@ -662,20 +665,21 @@ def buildStubs(): export.close() wrapper.close() - print "Generated %d wrapper functions" % nb_wrap + if not quiet: + print "Generated %d wrapper functions" % nb_wrap if unknown_types: print "Missing type converters: " for type in unknown_types.keys(): print "%s:%d " % (type, len(unknown_types[type])), - print - for f in functions_failed: print "ERROR: failed %s" % f if failed > 0: return -1 + if len(unknown_types) > 0: + return -1 return 0 ####################################################################### diff --git a/src/Makefile.am b/src/Makefile.am index 54b1ca0..875a43d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -796,7 +796,7 @@ endif BUILT_SOURCES += $(ESX_DRIVER_GENERATED) $(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input $(srcdir)/esx/esx_vi_generator.py - -srcdir=$(srcdir) $(srcdir)/esx/esx_vi_generator.py + $(AM_V_GEN)srcdir=$(srcdir) $(srcdir)/esx/esx_vi_generator.py if WITH_ESX if WITH_DRIVER_MODULES -- 1.7.6

3 5

[libvirt] [PATCH] tests: Fix virshtest failure after dominfo changed
by Osier Yang 19 Jul '11

19 Jul '11

Caused by the new changed "dominfo" command. Pushed under trival rule. --- tests/virshtest.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/tests/virshtest.c b/tests/virshtest.c index 0932efd..e22e582 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -22,6 +22,7 @@ Max memory: 261072 kB\n\ Used memory: 131072 kB\n\ Persistent: yes\n\ Autostart: disable\n\ +Managed save: unknown\n\ \n"; static const char *domuuid_fc4 = DOM_UUID "\n\n"; static const char *domid_fc4 = "2\n\n"; -- 1.7.6

1 0

[libvirt] [PATCH v2] util: avoid fds leak when virEventPollAddHandle fail
by ajia＠redhat.com 19 Jul '11

19 Jul '11

* src/util/event_poll.c: only fix file descriptors leak on when virEventPollAddHandle fail on virEventPollInit function. Detected in valgrind run: ==1254== ==1254== FILE DESCRIPTORS: 6 open at exit. ==1254== Open file descriptor 5: ==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82) ==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61) ==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648) ==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208) ==1254== by 0x42150C: main (virsh.c:13790) ==1254== ==1254== Open file descriptor 4: ==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82) ==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61) ==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648) ==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208) ==1254== by 0x42150C: main (virsh.c:13790) ==1254== * how to reproduce? % valgrind -v --track-fds=yes virsh list --- src/util/event_poll.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/util/event_poll.c b/src/util/event_poll.c index 285ba50..e2ae3a6 100644 --- a/src/util/event_poll.c +++ b/src/util/event_poll.c @@ -36,6 +36,7 @@ #include "event_poll.h" #include "memory.h" #include "util.h" +#include "files.h" #include "ignore-value.h" #include "virterror_internal.h" @@ -657,6 +658,8 @@ int virEventPollInit(void) virEventError(VIR_ERR_INTERNAL_ERROR, _("Unable to add handle %d to event loop"), eventLoop.wakeupfd[0]); + VIR_FORCE_CLOSE(eventLoop.wakeupfd[0]); + VIR_FORCE_CLOSE(eventLoop.wakeupfd[1]); return -1; } -- 1.7.1

3 2

Re: [libvirt] [libvirt-users] cannot perform tunnelled migration without using peer2peer flag
by Eric Blake 19 Jul '11

19 Jul '11

On 07/18/2011 04:11 AM, Osier Yang wrote: > 于 2011年07月18日 10:07, zhang xintao 写道: >> Dear All >> I try to migration a kvm guest os to another host failed >> server: ubuntu 11.04 server >> virsh:migrate --live --tunnelled vm1 qemu+ssh://192.168.10.3/system >> error:Requested operation is not valid:cannot perform >> tunnelled migration without using peer2peer flag > > The error tells you all, you need to use "--p2p". That said, why can't virsh be smarter, and automatically request the right underlying flags without making the user also type --p2p? Any problems with this patch? From db318d41c2a70189c21303c824aa4a862815874a Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake(a)redhat.com> Date: Mon, 18 Jul 2011 13:10:29 -0600 Subject: [PATCH] virsh: make migrate --tunnelled imply --p2p We can make the virsh migrate UI friendlier by supplying the missing bit automatically instead of erroring out when requesting --tunnelled without --p2p. * tools/virsh.c (doMigrate): Make --p2p optional when using --tunnelled. * tools/virsh.pod (migrate): Tweak wording accordingly. --- tools/virsh.c | 2 +- tools/virsh.pod | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 6c8032f..05aeb6c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4475,7 +4475,7 @@ doMigrate (void *opaque) if (vshCommandOptBool (cmd, "p2p")) flags |= VIR_MIGRATE_PEER2PEER; if (vshCommandOptBool (cmd, "tunnelled")) - flags |= VIR_MIGRATE_TUNNELLED; + flags |= VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER; if (vshCommandOptBool (cmd, "persistent")) flags |= VIR_MIGRATE_PERSIST_DEST; diff --git a/tools/virsh.pod b/tools/virsh.pod index e156232..e0adc31 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -534,14 +534,15 @@ Provide the maximum number of virtual CPUs supported for a guest VM on this connection. If provided, the I<type> parameter must be a valid type attribute for the <domain> element of XML. -=item B<migrate> [I<--live>] [I<--direct>] [I<--p2p> [I<--tunnelled>]] +=item B<migrate> [I<--live>] [I<--direct>] [I<--p2p>] [I<--tunnelled>] [I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>] [I<--copy-storage-inc>] [I<--verbose>] I<domain-id> I<desturi> [I<migrateuri>] [I<dname>] [I<--timeout> B<seconds>] Migrate domain to another host. Add I<--live> for live migration; I<--p2p> for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled> -for tunnelled migration. I<--persistent> leaves the domain persistent on +for tunnelled migration (I<--tunnelled> implies I<--p2p>). I<--persistent> +leaves the domain persistent on destination host, I<--undefinesource> undefines the domain on the source host, and I<--suspend> leaves the domain paused on the destination host. I<--copy-storage-all> indicates migration with non-shared storage with full -- 1.7.4.4 -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

3 6

[libvirt] [PATCH] rpc: Make the dispatch generator handle 'void name(void)' style procedures
by Matthias Bolte 19 Jul '11

19 Jul '11

Currently there are no such procedures in the protocol, but programming errors like a missing _args or _ret suffix on the structs in the .x files can create such a situation by accident. Making the generator aware of this avoids bogus errors from the generator such as: Use of uninitialized value in exists at ./rpc/gendispatch.pl line 967. Reported by Michal Privoznik --- src/rpc/gendispatch.pl | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index e068b53..e6a4a1c 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -143,6 +143,20 @@ while (<PROTOCOL>) { $flags = $3; $ProcName = name_to_ProcName ($name); + if (!exists $calls{$name}) { + # that the argument and return value cases have not yet added + # this procedure to the calls hash means that it has no arguments + # and no return value. add it to the calls hash now because all + # procedures have to be listed in the calls hash + $calls{$name} = { + name => $name, + ProcName => $ProcName, + UC_NAME => uc $name, + args => "void", + ret => "void" + } + } + if ($opt_b or $opt_k) { if (!($flags =~ m/^\s*\/\*\s*(\S+)\s+(\S+)\s*(.*)\*\/\s*$/)) { die "invalid generator flags for ${procprefix}_PROC_${name}" -- 1.7.4.1

2 1

[libvirt] [PATCH] util: avoid fds leak on virEventPollInit
by ajia＠redhat.com 19 Jul '11

19 Jul '11

* src/util/event_poll.c: fix file descriptors leak on virEventPollInit. Detected in valgrind run: ==1254== ==1254== FILE DESCRIPTORS: 6 open at exit. ==1254== Open file descriptor 5: ==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82) ==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61) ==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648) ==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208) ==1254== by 0x42150C: main (virsh.c:13790) ==1254== ==1254== Open file descriptor 4: ==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82) ==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61) ==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648) ==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208) ==1254== by 0x42150C: main (virsh.c:13790) ==1254== * how to reproduce? % valgrind -v --track-fds=yes virsh list --- src/util/event_poll.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/util/event_poll.c b/src/util/event_poll.c index 285ba50..1e4ef96 100644 --- a/src/util/event_poll.c +++ b/src/util/event_poll.c @@ -639,6 +639,8 @@ static void virEventPollHandleWakeup(int watch ATTRIBUTE_UNUSED, int virEventPollInit(void) { + int ret = -1; + if (virMutexInit(&eventLoop.lock) < 0) { virReportSystemError(errno, "%s", _("Unable to initialize mutex")); @@ -648,7 +650,7 @@ int virEventPollInit(void) if (pipe2(eventLoop.wakeupfd, O_CLOEXEC | O_NONBLOCK) < 0) { virReportSystemError(errno, "%s", _("Unable to setup wakeup pipe")); - return -1; + goto cleanup; } if (virEventPollAddHandle(eventLoop.wakeupfd[0], @@ -657,10 +659,15 @@ int virEventPollInit(void) virEventError(VIR_ERR_INTERNAL_ERROR, _("Unable to add handle %d to event loop"), eventLoop.wakeupfd[0]); - return -1; + goto cleanup; } - return 0; + ret = 0; + +cleanup: + close(eventLoop.wakeupfd[0]); + close(eventLoop.wakeupfd[1]); + return ret; } static int virEventPollInterruptLocked(void) -- 1.7.1

5 8

[libvirt] [PATCH v3] util: avoid fds leak when virEventPollAddHandle fail
by ajia＠redhat.com 19 Jul '11

19 Jul '11

* src/util/event_poll.c: avoid file descriptors leak when virEventPollAddHandle fail on virEventPollInit function. --- src/util/event_poll.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/util/event_poll.c b/src/util/event_poll.c index 285ba50..e2ae3a6 100644 --- a/src/util/event_poll.c +++ b/src/util/event_poll.c @@ -36,6 +36,7 @@ #include "event_poll.h" #include "memory.h" #include "util.h" +#include "files.h" #include "ignore-value.h" #include "virterror_internal.h" @@ -657,6 +658,8 @@ int virEventPollInit(void) virEventError(VIR_ERR_INTERNAL_ERROR, _("Unable to add handle %d to event loop"), eventLoop.wakeupfd[0]); + VIR_FORCE_CLOSE(eventLoop.wakeupfd[0]); + VIR_FORCE_CLOSE(eventLoop.wakeupfd[1]); return -1; } -- 1.7.1

1 0

[libvirt] [PATCH] interface: Check for interface (in-)activity on some operations
by Michal Privoznik 19 Jul '11

19 Jul '11

Right now it is possible to undefine an active interface, or destroy inactive. This patch add some checking to these operations to prevent this. Also fix test driver. --- src/interface/netcf_driver.c | 83 ++++++++++++++++++++++++++++++++++++------ src/test/test_driver.c | 5 +++ 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c index 855b5a3..bf590f3 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/netcf_driver.c @@ -119,6 +119,40 @@ static struct netcf_if *interfaceDriverGetNetcfIF(struct netcf *ncf, virInterfac return iface; } +/* + * interfaceObjIsActive: + * @iface - interface + * + * Caller MUST have driver locked to prevent race condition. + * Returns: + * -1 on error + * 0 on inactive interface + * 1 on active interface + */ +static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) +interfaceObjIsActive(struct interface_driver *driver, + struct netcf_if *iface, + virInterfacePtr ifinfo) +{ + int ret = -1; + unsigned int flags = 0; + + if (ncf_if_status(iface, &flags) < 0) { + const char *errmsg, *details; + int errcode = ncf_error(driver->netcf, &errmsg, &details); + interfaceReportError(netcf_to_vir_err(errcode), + _("failed to get status of interface %s: %s%s%s"), + ifinfo->name, errmsg, details ? " - " : "", + details ? details : ""); + goto cleanup; + } + + ret = flags & NETCF_IFACE_ACTIVE ? 1 : 0; + +cleanup: + return ret; +} + static virDrvOpenStatus interfaceOpenInterface(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) @@ -438,6 +472,7 @@ static int interfaceUndefine(virInterfacePtr ifinfo) { struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; int ret = -1; + int active; interfaceDriverLock(driver); @@ -447,6 +482,17 @@ static int interfaceUndefine(virInterfacePtr ifinfo) { goto cleanup; } + active = interfaceObjIsActive(driver, iface, ifinfo); + if (active < 0) { + /* helper already reported error */ + goto cleanup; + } else if (active) { + interfaceReportError(VIR_ERR_OPERATION_INVALID, + _("interface %s is active"), + ifinfo->name); + goto cleanup; + } + ret = ncf_if_undefine(iface); if (ret < 0) { const char *errmsg, *details; @@ -470,6 +516,7 @@ static int interfaceCreate(virInterfacePtr ifinfo, struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; int ret = -1; + int active; virCheckFlags(0, -1); @@ -481,6 +528,17 @@ static int interfaceCreate(virInterfacePtr ifinfo, goto cleanup; } + active = interfaceObjIsActive(driver, iface, ifinfo); + if (active < 0) { + /* helper already reported error */ + goto cleanup; + } else if (active) { + interfaceReportError(VIR_ERR_OPERATION_INVALID, + _("interface %s is already active"), + ifinfo->name); + goto cleanup; + } + ret = ncf_if_up(iface); if (ret < 0) { const char *errmsg, *details; @@ -504,6 +562,7 @@ static int interfaceDestroy(virInterfacePtr ifinfo, struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; int ret = -1; + int active; virCheckFlags(0, -1); @@ -515,6 +574,17 @@ static int interfaceDestroy(virInterfacePtr ifinfo, goto cleanup; } + active = interfaceObjIsActive(driver, iface, ifinfo); + if (active < 0) { + /* helper already reported error */ + goto cleanup; + } else if (!active) { + interfaceReportError(VIR_ERR_OPERATION_INVALID, + _("interface %s is not active"), + ifinfo->name); + goto cleanup; + } + ret = ncf_if_down(iface); if (ret < 0) { const char *errmsg, *details; @@ -536,7 +606,6 @@ static int interfaceIsActive(virInterfacePtr ifinfo) { struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; - unsigned int flags = 0; int ret = -1; interfaceDriverLock(driver); @@ -547,17 +616,7 @@ static int interfaceIsActive(virInterfacePtr ifinfo) goto cleanup; } - if (ncf_if_status(iface, &flags) < 0) { - const char *errmsg, *details; - int errcode = ncf_error(driver->netcf, &errmsg, &details); - interfaceReportError(netcf_to_vir_err(errcode), - _("failed to get status of interface %s: %s%s%s"), - ifinfo->name, errmsg, details ? " - " : "", - details ? details : ""); - goto cleanup; - } - - ret = flags & NETCF_IFACE_ACTIVE ? 1 : 0; + ret = interfaceObjIsActive(driver, iface, ifinfo); cleanup: ncf_if_free(iface); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f3fb320..d165586 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3648,6 +3648,11 @@ static int testInterfaceUndefine(virInterfacePtr iface) goto cleanup; } + if (privinterface->active != 0) { + testError(VIR_ERR_OPERATION_INVALID, NULL); + goto cleanup; + } + virInterfaceRemove(&privconn->ifaces, privinterface); ret = 0; -- 1.7.5.rc3

4 10

[libvirt] [PATCH v2 0/8] Introduce New API virDomainUndefineFlags
by Osier Yang 19 Jul '11

19 Jul '11

Per discussion on https://www.redhat.com/archives/libvir-list/2011-July/msg00556.html, This patch series introduce new API virDomainUndefineFlags, which only support flag VIR_DOMAIN_UNDEFINE_MANAGED_STATE now, might introduce more flags in future though. If the domain has a managed state file, then including VIR_DOMAIN_UNDEFINE_MANAGED_STATE in @flags will also remove that file, and omitting the flag will cause the undefine process to fail. This also changes the behavior of virDomainUndefine, but considering undefine a domain with managed state file left is unsafe, we need to do so. [PATCH v2 1/8] is small fix on rpc generator scripts, not related with the new API. [PATCH v2 1/8] rpc: Fix typos in rpc generator scripts [PATCH v2 2/8] undefine: Define the new API [PATCH v2 3/8] undefine: Wire up the remote protocol [PATCH v2 4/8] undefine: Implement internal API for qemu driver [PATCH v2 5/8] undefine: Implement internal API for libxl driver [PATCH v2 6/8] undefine: Implement undefineFlags in all other [PATCH v2 7/8] undefine: Extend virsh undefine to support the new [PATCH v2 8/8] virsh: Extend virsh dominfo to display if managed v1 - v2: * s/virDomainUndefineWithFlags/virDomainUndefineFlags/ * Update with other bunch of feedbacks from Eric * Changes on virsh dominfo to display if domain managed state exists Regards Osier

2 31

[libvirt] [PATCH] doc: Correct docs for iface commands
by Osier Yang 19 Jul '11

19 Jul '11

duplicate documents for "iface-name", lacks of document for "iface-mac", inconsistent option names with virsh help strings. --- tools/virsh.pod | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 377fac0..4f218d3 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1081,20 +1081,24 @@ Returns the list of active host interfaces. If I<--all> is specified this will also include defined but inactive interfaces. If I<--inactive> is specified only the inactive ones will be listed. -=item B<iface-name> I<iface-MAC> +=item B<iface-name> I<interface> -Convert a host interface MAC to interface name, if the I<iface-MAC> is -unique among the host's interfaces. +Convert a host interface MAC to interface name, if the MAC address is unique +among the host's interfaces. -=item B<iface-name> I<iface-MAC> +I<interface> specifies the interface MAC address. + +=item B<iface-mac> I<interface> Convert a host interface name to MAC address. -=item B<iface-start> I<iface> +I<interface> specifies the interface name. + +=item B<iface-start> I<interface> Start a (previously defined) host interface, such as by running "if-up". -=item B<iface-undefine> I<iface> +=item B<iface-undefine> I<interface> Undefine the configuration for an inactive host interface. -- 1.7.6

2 2