May 2010 - Devel - libvirt List Archives

[libvirt] [0/2] Fix 2 possible crashes in JSON event dispatch
by Daniel P. Berrange 11 May '10

11 May '10

A couple of places didn't do sufficient NULL checks in handling dispatch of JSON events, so can crash under certain conditions.

2 4

[libvirt] [PATCH] virsh: schedinfo --set invalid=value would simply ignore the option
by Jim Meyering 11 May '10

11 May '10

Currently, virsh does this: $ virsh -c test:///default schedinfo 1 --set j=k>/dev/null && echo bad bad It should have diagnosed "j=k" as an invalid schedinfo --set argument. With the patch below, it does: $ ./virsh -c test:///default schedinfo 1 --set j=k>/dev/null && echo bad error: invalid scheduler option: j=k [Exit 1] >From cf56648333c4e5cf73ab3292ba727e33b0ae9708 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 11 May 2010 15:38:21 +0200 Subject: [PATCH] virsh: schedinfo --set invalid=value would simply ignore the option For example, virsh -c test:///default schedinfo 1 --set P=k would mistakenly exit successfully, giving no indication that it had failed to set the scheduling parameter "P". * tools/virsh.c (cmdSchedinfo): Diagnose an invalid --set j=k option, rather than silently ignoring it. * tests/virsh-schedinfo: New test for the above. * tests/Makefile.am (test_scripts): Add it. Reported by Jintao Yang in http://bugzilla.redhat.com/586632 --- tests/Makefile.am | 1 + tests/virsh-schedinfo | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.c | 10 ++++++++++ 3 files changed, 60 insertions(+), 0 deletions(-) create mode 100755 tests/virsh-schedinfo diff --git a/tests/Makefile.am b/tests/Makefile.am index ef12386..b5e09e3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -139,6 +139,7 @@ test_scripts += \ undefine \ vcpupin \ virsh-all \ + virsh-schedinfo \ virsh-synopsis endif diff --git a/tests/virsh-schedinfo b/tests/virsh-schedinfo new file mode 100755 index 0000000..116014a --- /dev/null +++ b/tests/virsh-schedinfo @@ -0,0 +1,49 @@ +#!/bin/sh +# Ensure that virsh schedinfo --set invalid=val fails + +# Copyright (C) 2010 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program 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 General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +: ${srcdir=$(pwd)} +: ${abs_top_srcdir=$(pwd)/..} +: ${abs_top_builddir=$(pwd)/..} + +# If $abs_top_builddir/tools/virsh is not early in $PATH, put it there, +# so that we can safely invoke "virsh" simply with its name. +case $PATH in + $abs_top_builddir/tools/src:$abs_top_builddir/tools/virsh:*) ;; + $abs_top_builddir/tools/virsh:*) ;; + *) PATH=$abs_top_builddir/tools/virsh:$PATH; export PATH ;; +esac + +if test "$VERBOSE" = yes; then + set -x + $abs_top_builddir/tools/virsh --version +fi + +. "$srcdir/test-lib.sh" + +printf 'Scheduler : fair\n\n' > exp-out || framework_failure +printf 'error: invalid scheduler option: j=k\n' > exp-err || framework_failure + +fail=0 + +test_url=test:///default + +virsh -c $test_url schedinfo 1 --set j=k >out 2>err && fail=1 +compare out exp-out || fail=1 +compare err exp-err || fail=1 + +(exit $fail); exit $fail diff --git a/tools/virsh.c b/tools/virsh.c index 0a63f1b..99f383a 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1594,6 +1594,16 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) ret = virDomainGetSchedulerParameters(dom, params, &nparams); if (ret == -1) goto cleanup; + } else { + /* See if we've tried to --set var=val. If so, the fact that + we reach this point (with update == 0) means that "var" did + not match any of the settable parameters. Report the error. */ + char *var_value_pair = vshCommandOptString(cmd, "set", NULL); + if (var_value_pair) { + vshError(ctl, _("invalid scheduler option: %s"), + var_value_pair); + goto cleanup; + } } ret_val = TRUE; -- 1.7.1.189.g07419

3 3

[libvirt] [PATCH] virsh: fix a typo in a diagnostic
by Jim Meyering 11 May '10

11 May '10

FYI, I've just pushed these two typo fixes: >From 5603515127ff14b70bd6a24db1485043a6b817a7 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 11 May 2010 16:02:28 +0200 Subject: [PATCH] virsh: fix a typo in a diagnostic * tools/virsh.c (cmdSchedInfoUpdate): Fix typo in a diagnostic: s/an long long/a long long/. One in a comment, too. --- tools/virsh.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 0a63f1b..b9bf06d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -88,7 +88,7 @@ static char *progname; /** * vshErrorLevel: * - * Indicates the level of an log message + * Indicates the level of a log message */ typedef enum { VSH_ERR_DEBUG = 0, @@ -1510,7 +1510,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, case VIR_DOMAIN_SCHED_FIELD_LLONG: if (virStrToLong_ll(val, NULL, 10, &param->value.l) < 0) { vshError(ctl, "%s", - _("Invalid value for parameter, expecting an long long")); + _("Invalid value for parameter, expecting a long long")); return -1; } break; -- 1.7.1.189.g07419

1 0

[libvirt] Build failure on RHEL-5
by Chris Lalancette 11 May '10

11 May '10

Hello, My autobuild of libvirt on RHEL-5 failed last night because of: if gcc -DHAVE_CONFIG_H -I. -I. -I.. -I../gnulib/lib -I../gnulib/lib -I../include -I../include -I../src -I../src/util -I../src/conf -I../src/remote -I/usr/include/libxml2 -Wall -Wformat -Wformat-security -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wextra -Wshadow -Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Winline -Wredundant-decls -Wno-sign-compare -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fasynchronous-unwind-tables -fdiagnostics-show-option -Werror -DLOCAL_STATE_DIR="\"/usr/local/var\"" -DSYSCONF_DIR="\"/usr/local/etc\"" -DQEMUD_PID_FILE="\"\"" -DREMOTE_PID_FILE="\"/usr/local/var/run/libvirtd.pid\"" -DGETTEXT_PACKAGE=\"libvirt\" -D_REENTRANT -g -O2 -MT libvirtd-dispatch.o -MD -MP -MF ".deps/libvirtd-dispatch.Tpo" -c -o libvirtd-dispatch.o `test -f 'dispatch.c' || echo './'`dispatch.c; \ then mv -f ".deps/libvirtd-dispatch.Tpo" ".deps/libvirtd-dispatch.Po"; else rm -f ".deps/libvirtd-dispatch.Tpo"; exit 1; fi cc1: warnings being treated as errors dispatch.c: In function 'remoteDispatchClientCall': dispatch.c:444: warning: value computed is not used Git head revision: 78a6af1ff9cdbb799f9e02d1f0160b2f9ee61cd9 >From a quick look, I can't quite see what it is complaining about; args is clearly used later in the function, so it's kind of weird. Anyway, if anyone has time to look, it's probably something we should fix. -- Chris Lalancette

1 0

[libvirt] [PATCH] docs/Makefile.am: remove unnecessary subshells
by Eric Blake 11 May '10

11 May '10

* docs/Makefile.am (ChangeLog.html.in, %.html.tmp, %.html) (html/index.html, $(devhelphtml)): Avoid spurious subshells. --- I'm still having problems getting a VPATH build to work correctly, but noticed this low-hanging fruit in the process. Reformatting to 80 columns is a nice side effect. docs/Makefile.am | 35 ++++++++++++++++++++--------------- 1 files changed, 20 insertions(+), 15 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index a18821b..70b9e51 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -91,44 +91,49 @@ ChangeLog.xml: ../ChangeLog ChangeLog.awk awk -f ChangeLog.awk < $< > $@ ChangeLog.html.in: ChangeLog.xml ChangeLog.xsl - @(if [ -x $(XSLTPROC) ] ; then \ + @if [ -x $(XSLTPROC) ] ; then \ echo "Generating $@"; \ name=`echo $@ | sed -e 's/.tmp//'`; \ - $(XSLTPROC) --nonet $(top_srcdir)/docs/ChangeLog.xsl $< > $@ || (rm $@ && exit 1) ; fi ) + $(XSLTPROC) --nonet $(top_srcdir)/docs/ChangeLog.xsl $< > $@ \ + || { rm $@ && exit 1; }; fi %.png: %.fig convert -rotate 90 $< $@ %.html.tmp: %.html.in site.xsl page.xsl sitemap.html.in - @(if [ -x $(XSLTPROC) ] ; then \ + @if [ -x $(XSLTPROC) ] ; then \ echo "Generating $@"; \ name=`echo $@ | sed -e 's/.tmp//'`; \ - $(XSLTPROC) --stringparam pagename $$name --nonet --html $(top_srcdir)/docs/site.xsl $< > $@ || (rm $@ && exit 1) ; fi ) + $(XSLTPROC) --stringparam pagename $$name --nonet --html \ + $(top_srcdir)/docs/site.xsl $< > $@ \ + || { rm $@ && exit 1; }; fi %.html: %.html.tmp - @(if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \ - if $(XMLCATALOG) /etc/xml/catalog "-//W3C//DTD XHTML 1.0 Strict//EN" > /dev/null ; then \ + @if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \ + if $(XMLCATALOG) /etc/xml/catalog \ + "-//W3C//DTD XHTML 1.0 Strict//EN" > /dev/null ; then \ echo "Validating $@" ; \ - $(XMLLINT) --nonet --format --valid $< > $@ || (rm $@ && exit 1) ; \ - else echo "missing XHTML1 DTD" ; fi ; fi ); + $(XMLLINT) --nonet --format --valid $< > $@ \ + || { rm $@ && exit 1; }; \ + else echo "missing XHTML1 DTD" ; fi ; fi html/index.html: libvirt-api.xml newapi.xsl page.xsl sitemap.html.in - -@(if [ -x $(XSLTPROC) ] ; then \ + -@if [ -x $(XSLTPROC) ] ; then \ echo "Rebuilding the HTML pages from the XML API" ; \ - $(XSLTPROC) --nonet $(srcdir)/newapi.xsl libvirt-api.xml ; fi ) - -@(if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \ + $(XSLTPROC) --nonet $(srcdir)/newapi.xsl libvirt-api.xml ; fi + -@if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \ if $(XMLCATALOG) /etc/xml/catalog "-//W3C//DTD XHTML 1.0 Strict//EN" \ > /dev/null ; then \ echo "Validating the resulting XHTML pages" ; \ $(XMLLINT) --nonet --valid --noout html/*.html ; \ - else echo "missing XHTML1 DTD" ; fi ; fi ); + else echo "missing XHTML1 DTD" ; fi ; fi $(devhelphtml): libvirt-api.xml $(devhelpxsl) - -@(echo Rebuilding devhelp files) - -@(if [ -x $(XSLTPROC) ] ; then \ + -@echo Rebuilding devhelp files + -@if [ -x $(XSLTPROC) ] ; then \ $(XSLTPROC) --nonet -o devhelp/libvirt.devhelp \ - $(top_srcdir)/docs/devhelp/devhelp.xsl libvirt-api.xml ; fi ); + $(top_srcdir)/docs/devhelp/devhelp.xsl libvirt-api.xml ; fi html/%-%.html html/%-virterror.html %-api.xml %-refs.xml: $(srcdir)/apibuild.py -srcdir=$(srcdir) $(srcdir)/apibuild.py -- 1.6.6.1

2 1

[libvirt] locking down struct size/layout in remote-protocol.x
by Jim Meyering 11 May '10

11 May '10

This week we noticed that a change to struct remote_error was causing trouble (new member addition changed the size of the object being decoded -- bad for the protocol). In order to ensure that no such changes sneak in again, I'm planning to do the following. First, count how many structs must not change size or layout: $ grep -E '^struct remote_' src/*/remote_protocol.x|wc -l 318 Obviously not appropriate to do it manually. Running this (pdwtags is part of the dwarves package): pdwtags src/libvirt_driver_remote_la-remote_protocol.o prints, among other things, detailed type info for every struct: /* 89 */ struct remote_nonnull_domain { remote_nonnull_string name; /* 0 8 */ remote_uuid uuid; /* 8 16 */ int id; /* 24 4 */ /* size: 32, cachelines: 1, members: 3 */ /* last cacheline: 32 bytes */ /* BRAIN FART ALERT! 32 != 28 + 0(holes), diff = 4 */ }; /* size: 32 */ /* 90 */ typedef struct remote_nonnull_domain remote_nonnull_domain; /* size: 32 */ /* 91 */ struct remote_nonnull_network { remote_nonnull_string name; /* 0 8 */ remote_uuid uuid; /* 8 16 */ /* size: 24, cachelines: 1, members: 2 */ /* last cacheline: 24 bytes */ }; /* size: 24 */ ----------------------------------------------------- A) For each of the above matching /^struct remote_/, print at the very least this sort of compile-time check: verify (sizeof (struct remote_nonnull_domain) == 32); verify (sizeof (struct remote_nonnull_network) == 24); B) Even better, verify member offsets, so that we'd detect reordered members (which often will not change struct size): verify (sizeof (struct remote_nonnull_domain) == 32); verify (offsetof (struct remote_nonnull_domain, name) == 0); verify (offsetof (struct remote_nonnull_domain, uuid) == 8); verify (offsetof (struct remote_nonnull_domain, id) == 24); verify (sizeof (struct remote_nonnull_network) == 24); verify (offsetof (struct remote_nonnull_network, name) == 0); verify (offsetof (struct remote_nonnull_network, uuid) == 16); C) Might as well verify member sizes as well. ----------------------------------------------------- Once we have a small script to generate that (RSN), include the generated, (and version-controlled) file from remote_protocol.c, and we're done. For reference, here's code to generate A): pdwtags src/libvirt_driver_remote_la-remote_protocol.o \ | perl -0777 -n \ -e 'foreach my $p (split m!\n\n/\* \d+ \*/\n!)' \ -e ' { $p =~ m!^struct (remote_\w+).*/\* size: (\d+) \*/!s' \ -e ' and print "verify (sizeof (struct $1) == $2);\n" }' And here's a sample of its output: verify (sizeof (struct remote_nonnull_domain) == 32); verify (sizeof (struct remote_nonnull_network) == 24); verify (sizeof (struct remote_nonnull_nwfilter) == 24); verify (sizeof (struct remote_nonnull_interface) == 16); verify (sizeof (struct remote_nonnull_storage_pool) == 24); verify (sizeof (struct remote_nonnull_storage_vol) == 24); verify (sizeof (struct remote_nonnull_node_device) == 8); ... verify (sizeof (struct remote_domain_has_current_snapshot_args) == 40); verify (sizeof (struct remote_domain_has_current_snapshot_ret) == 4); verify (sizeof (struct remote_domain_snapshot_current_args) == 40); verify (sizeof (struct remote_domain_snapshot_current_ret) == 40); verify (sizeof (struct remote_domain_revert_to_snapshot_args) == 48); verify (sizeof (struct remote_domain_snapshot_delete_args) == 48); verify (sizeof (struct remote_message_header) == 24);

3 15

[libvirt] [PATCH] delMacvtap: typo fix
by Eric Blake 11 May '10

11 May '10

* src/util/macvtap.c (delMacvtap): Fix documentation. --- Pushing as trivial. src/util/macvtap.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 5d129fd..1f8dd29 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -669,13 +669,13 @@ link_del_exit: return rc; } /** - * delMacvtapByName: + * delMacvtap: * @ifname : The name of the macvtap interface * * Delete an interface given its name. */ void delMacvtap(const char *ifname) -- 1.7.0.1

2 1

[libvirt] [PATCH] maint: allow VPATH use of remote_protocol-structs
by Eric Blake 10 May '10

10 May '10

* src/Makefile.am (remote_protocol-structs): Ensure file lives in srcdir. --- Jim's new check didn't quite work in a VPATH setup. I'm applying it as obvious, since it was a build-breaker: GEN remote_protocol-structs diff: remote_protocol-structs: No such file or directory make[3]: *** [remote_protocol-structs] Error 2 src/Makefile.am | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 1542ab1..15bc8fc 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -179,7 +179,7 @@ remote_protocol-structs: -e ' $$p =~ s!\s+$$!!;' \ -e ' print "$$p\n" } }' \ > $@-t; \ - diff -u $@-t $@; st=$$?; rm -f $@-t; exit $$st; \ + diff -u $@-t $(srcdir)/$@; st=$$?; rm -f $@-t; exit $$st; \ else \ echo 'WARNING: you lack pdwtags; skipping the $@ test'; \ echo 'WARNING: install the dwarves package to get pdwtags'; \ -- 1.7.0.1

1 0

[libvirt] [PATCH] build: fix autogen rule for VPATH build
by Eric Blake 10 May '10

10 May '10

* cfg.mk (gnulib_srcdir): Override maint.mk default. (_update_required): Run in correct directory. --- I noticed an error message about bootstrap.conf not found when trying to do a VPATH build for my new clang setup. This subsumes up my earlier patch to declare the correct gnulib_srcdir, as noticed by Jim, but does not advance to a newer .gnulib, as there are some other issues to fix on that front first. cfg.mk | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/cfg.mk b/cfg.mk index 105b625..a6e9204 100644 --- a/cfg.mk +++ b/cfg.mk @@ -24,6 +24,9 @@ gnu_rel_host = $(gnu_ftp_host-$(RELEASE_TYPE)) url_dir_list = \ ftp://$(gnu_rel_host)/gnu/coreutils +# We use .gnulib, not gnulib. +gnulib_srcdir = $(srcdir)/.gnulib + # Tests not to run as part of "make distcheck". local-checks-to-skip = \ changelog-check \ @@ -282,6 +285,7 @@ ifeq (0,$(MAKELEVEL)) # b653eda3ac4864de205419d9f41eec267cb89eeb _submodule_hash = sed 's/^[ +-]//;s/ .*//' _update_required := $(shell \ + cd '$(srcdir)'; \ actual=$$(git submodule status | $(_submodule_hash); \ git hash-object bootstrap.conf; \ git diff .gnulib); \ -- 1.6.6.1

4 18

[libvirt] question on network interface of LXC domain
by Osier Yang 10 May '10

10 May '10

Hello there with newest libvirt, when I connect to the container via console, then execute "ifconfig", there will be no output, though the network works fine. and "/proc/net/dev" of container seems good too. (has entries about "eth0" and "lo"). I traced the call procedure of "ifconfig", it reads the infomations of network interface from "/proc/net/dev" and vise visa. so I'm confused about what cause this problem. there is no problem with libvirt-0.8.0-4. Thanks and Regards Osier

2 1