[libvirt] [PATCH] virsh: schedinfo --set invalid=value would simply ignore the option
by Jim Meyering
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
14 years, 11 months
[libvirt] [PATCH] virsh: fix a typo in a diagnostic
by Jim Meyering
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, ¶m->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
14 years, 11 months
[libvirt] Build failure on RHEL-5
by Chris Lalancette
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
14 years, 11 months
[libvirt] [PATCH] docs/Makefile.am: remove unnecessary subshells
by Eric Blake
* 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
14 years, 11 months
[libvirt] locking down struct size/layout in remote-protocol.x
by Jim Meyering
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);
14 years, 11 months
[libvirt] [PATCH] delMacvtap: typo fix
by Eric Blake
* 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
14 years, 11 months
[libvirt] [PATCH] maint: allow VPATH use of remote_protocol-structs
by Eric Blake
* 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
14 years, 11 months
[libvirt] [PATCH] build: fix autogen rule for VPATH build
by Eric Blake
* 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
14 years, 11 months
[libvirt] question on network interface of LXC domain
by Osier Yang
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
14 years, 11 months
