[libvirt] [PATCH v2] qemu_agent: Report error class at least
by Michal Privoznik
Currently, qemu GA is not providing 'desc' field for errors like
we are used to from qemu monitor. Therefore, we fall back to this
general 'unknown error' string. However, GA is reporting 'class' which
is not perfect, but much more helpful than generic error string.
Thus we should fall back to class firstly and if even no class
is presented, then we can fall back to that generic string.
Before this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': unknown QEMU command error
After this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': The command has not been found
---
src/qemu/qemu_agent.c | 37 +++++++++++++++++++++++++++++++++++--
1 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index b759b7f..bc4ceff 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1025,6 +1025,40 @@ cleanup:
return ret;
}
+static const char *
+qemuAgentStringifyErrorClass(const char *klass)
+{
+ if (STREQ_NULLABLE(klass, "BufferOverrun"))
+ return "Buffer overrun";
+ else if (STREQ_NULLABLE(klass, "CommandDisabled"))
+ return "The command has been disabled for this instance";
+ else if (STREQ_NULLABLE(klass, "CommandNotFound"))
+ return "The command has not been found";
+ else if (STREQ_NULLABLE(klass, "FdNotFound"))
+ return "File descriptor not found";
+ else if (STREQ_NULLABLE(klass, "InvalidParameter"))
+ return "Invalid parameter";
+ else if (STREQ_NULLABLE(klass, "InvalidParameterType"))
+ return "Invalid parameter type";
+ else if (STREQ_NULLABLE(klass, "InvalidParameterValue"))
+ return "Invalid parameter value";
+ else if (STREQ_NULLABLE(klass, "OpenFileFailed"))
+ return "Cannot open file";
+ else if (STREQ_NULLABLE(klass, "QgaCommandFailed"))
+ return "Guest agent command failed";
+ else if (STREQ_NULLABLE(klass, "QMPBadInputObjectMember"))
+ return "Bad QMP input object member";
+ else if (STREQ_NULLABLE(klass, "QMPExtraInputObjectMember"))
+ return "Unexpected extra object member";
+ else if (STREQ_NULLABLE(klass, "UndefinedError"))
+ return "An undefined error has occurred";
+ else if (STREQ_NULLABLE(klass, "Unsupported"))
+ return "this feature or command is not currently supported";
+ else if (klass)
+ return klass;
+ else
+ return "unknown QEMU command error";
+}
/* Ignoring OOM in this method, since we're already reporting
* a more important error
@@ -1043,9 +1077,8 @@ qemuAgentStringifyError(virJSONValuePtr error)
if (klass)
detail = virJSONValueObjectGetString(error, "desc");
-
if (!detail)
- detail = "unknown QEMU command error";
+ detail = qemuAgentStringifyErrorClass(klass);
return detail;
}
--
1.7.8.5
13 years
[libvirt] [PATCH] More coverity findings addressed
by Stefan Berger
More bug extermination in the category of:
Error: CHECKED_RETURN:
/libvirt/src/conf/network_conf.c:595:
check_return: Calling function "virAsprintf" without checking return
value (as is done elsewhere 515 out of 543 times).
/libvirt/src/qemu/qemu_process.c:2780:
unchecked_value: No check of the return value of "virAsprintf(&msg, "was
paused (%s)", virDomainPausedReasonTypeToString(reason))".
/libvirt/tests/commandtest.c:809:
check_return: Calling function "setsid" without checking return value
(as is done elsewhere 4 out of 5 times).
/libvirt/tests/commandtest.c:830:
unchecked_value: No check of the return value of "virTestGetDebug()".
/libvirt/tests/commandtest.c:831:
check_return: Calling function "virTestGetVerbose" without checking
return value (as is done elsewhere 41 out of 42 times).
/libvirt/tests/commandtest.c:833:
check_return: Calling function "virInitialize" without checking return
value (as is done elsewhere 18 out of 21 times).
One note about the error in commandtest line 809: setsid() seems to fail
when running the test -- could be removed ?
---
src/conf/network_conf.c | 7 ++-----
src/qemu/qemu_process.c | 4 ++--
tests/commandtest.c | 9 +++++----
3 files changed, 9 insertions(+), 11 deletions(-)
Index: libvirt-acl/src/conf/network_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/network_conf.c
+++ libvirt-acl/src/conf/network_conf.c
@@ -590,12 +590,9 @@ virNetworkDNSSrvDefParseXML(virNetworkDN
}
if (strlen(service) > DNS_RECORD_LENGTH_SRV) {
- char *name = NULL;
-
- virAsprintf(&name, _("Service name is too long, limit is %d
bytes"), DNS_RECORD_LENGTH_SRV);
virNetworkReportError(VIR_ERR_XML_DETAIL,
- "%s", name);
- VIR_FREE(name);
+ _("Service name is too long, limit is %d
bytes"),
+ DNS_RECORD_LENGTH_SRV);
goto error;
}
Index: libvirt-acl/src/qemu/qemu_process.c
===================================================================
--- libvirt-acl.orig/src/qemu/qemu_process.c
+++ libvirt-acl/src/qemu/qemu_process.c
@@ -2777,8 +2777,8 @@ qemuProcessUpdateState(struct qemud_driv
} else {
newState = VIR_DOMAIN_PAUSED;
newReason = reason;
- virAsprintf(&msg, "was paused (%s)",
- virDomainPausedReasonTypeToString(reason));
+ ignore_value(virAsprintf(&msg, "was paused (%s)",
+
virDomainPausedReasonTypeToString(reason)));
}
} else if (state == VIR_DOMAIN_SHUTOFF && running) {
newState = VIR_DOMAIN_RUNNING;
Index: libvirt-acl/tests/commandtest.c
===================================================================
--- libvirt-acl.orig/tests/commandtest.c
+++ libvirt-acl/tests/commandtest.c
@@ -806,7 +806,7 @@ mymain(void)
return EXIT_FAILURE;
setpgid(0, 0);
- setsid();
+ ignore_value(setsid());
/* Our test expects particular fd values; to get that, we must not
* leak fds that we inherited from a lazy parent. At the same
@@ -827,10 +827,11 @@ mymain(void)
/* Prime the debug/verbose settings from the env vars,
* since we're about to reset 'environ' */
- virTestGetDebug();
- virTestGetVerbose();
+ ignore_value(virTestGetDebug());
+ ignore_value(virTestGetVerbose());
- virInitialize();
+ if (virInitialize() < 0)
+ return EXIT_FAILURE;
/* Phase two of killing interfering fds; see above. */
fd = 3;
13 years
[libvirt] [PATCHv2] blockjob: fix block-stream bandwidth race
by Eric Blake
With RHEL 6.2, virDomainBlockPull(dom, dev, bandwidth, 0) has a race
with non-zero bandwidth: there is a window between the block_stream
and block_job_set_speed monitor commands where an unlimited amount
of data was let through, defeating the point of a throttle.
This race was first identified in commit a9d3495e, and libvirt was
able to reduce the size of the window for that race. In the meantime,
the qemu developers decided to fix things properly; per this message:
https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg03793.html
the fix will be in qemu 1.1, and changes block-job-set-speed to use
a different parameter name, as well as adding a new optional parameter
to block-stream, which eliminates the race altogether.
Since our documentation already mentioned that we can refuse a non-zero
bandwidth for some hypervisors, I think the best solution is to do
just that for RHEL 6.2 qemu, so that the race is obvious to the user
(anyone using stock RHEL 6.2 binaries won't have this patch, and anyone
building their own libvirt with this patch for RHEL can also rebuild
qemu to get the modern semantics, so it is no real loss in behavior).
Meanwhile the code must be fixed to honor actual qemu 1.1 naming.
Rename the parameter to 'modern', since the naming difference now
covers more than just 'async' block-job-cancel. And while at it,
fix an unchecked integer overflow.
* src/qemu/qemu_monitor.h (enum BLOCK_JOB_CMD): Drop unused value,
rename enum to match conventions.
* src/qemu/qemu_monitor.c (qemuMonitorBlockJob): Reflect enum rename.
* src/qemu_qemu_monitor_json.h (qemuMonitorJSONBlockJob): Likewise.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockJob): Likewise,
and support difference between RHEL 6.2 and qemu 1.1 block pull.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Reject
bandwidth during pull with too-old qemu.
* src/libvirt.c (virDomainBlockPull, virDomainBlockRebase):
Document this.
---
v2: fix integer overflow, improve variable naming
src/libvirt.c | 8 ++++-
src/qemu/qemu_driver.c | 8 +++--
src/qemu/qemu_monitor.c | 25 +++++++++++++-----
src/qemu/qemu_monitor.h | 15 +++++-----
src/qemu/qemu_monitor_json.c | 59 +++++++++++++++++++++++------------------
src/qemu/qemu_monitor_json.h | 6 ++--
6 files changed, 72 insertions(+), 49 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index b01ebba..cfd7711 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -18145,7 +18145,9 @@ error:
* The maximum bandwidth (in Mbps) that will be used to do the copy can be
* specified with the bandwidth parameter. If set to 0, libvirt will choose a
* suitable default. Some hypervisors do not support this feature and will
- * return an error if bandwidth is not 0.
+ * return an error if bandwidth is not 0; in this case, it might still be
+ * possible for a later call to virDomainBlockJobSetSpeed() to succeed.
+ * The actual speed can be determined with virDomainGetBlockJobInfo().
*
* This is shorthand for virDomainBlockRebase() with a NULL base.
*
@@ -18263,7 +18265,9 @@ error:
* The maximum bandwidth (in Mbps) that will be used to do the copy can be
* specified with the bandwidth parameter. If set to 0, libvirt will choose a
* suitable default. Some hypervisors do not support this feature and will
- * return an error if bandwidth is not 0.
+ * return an error if bandwidth is not 0; in this case, it might still be
+ * possible for a later call to virDomainBlockJobSetSpeed() to succeed.
+ * The actual speed can be determined with virDomainGetBlockJobInfo().
*
* When @base is NULL and @flags is 0, this is identical to
* virDomainBlockPull().
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c3555ca..d3aa34d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11654,6 +11654,11 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
_("partial block pull not supported with this "
"QEMU binary"));
goto cleanup;
+ } else if (mode == BLOCK_JOB_PULL && bandwidth) {
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("setting bandwidth at start of block pull not "
+ "supported with this QEMU binary"));
+ goto cleanup;
}
device = qemuDiskPathToAlias(vm, path, &idx);
@@ -11676,9 +11681,6 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
* relying on qemu to do this. */
ret = qemuMonitorBlockJob(priv->mon, device, base, bandwidth, info, mode,
async);
- if (ret == 0 && mode == BLOCK_JOB_PULL && bandwidth)
- ret = qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
- BLOCK_JOB_SPEED_INTERNAL, async);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret < 0)
goto endjob;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2f66c46..ca1c7aa 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2768,23 +2768,34 @@ int qemuMonitorScreendump(qemuMonitorPtr mon,
return ret;
}
+/* bandwidth is in MB/sec */
int qemuMonitorBlockJob(qemuMonitorPtr mon,
const char *device,
const char *base,
unsigned long bandwidth,
virDomainBlockJobInfoPtr info,
- int mode,
- bool async)
+ qemuMonitorBlockJobCmd mode,
+ bool modern)
{
int ret = -1;
+ unsigned long long speed;
- VIR_DEBUG("mon=%p, device=%s, base=%s, bandwidth=%lu, info=%p, mode=%o, "
- "async=%d", mon, device, NULLSTR(base), bandwidth, info, mode,
- async);
+ VIR_DEBUG("mon=%p, device=%s, base=%s, bandwidth=%luM, info=%p, mode=%o, "
+ "modern=%d", mon, device, NULLSTR(base), bandwidth, info, mode,
+ modern);
+
+ /* Convert bandwidth MiB to bytes */
+ if (bandwidth > ULLONG_MAX / 1024 / 1024) {
+ qemuReportError(VIR_ERR_OVERFLOW,
+ _("bandwidth must be less than %llu"),
+ ULLONG_MAX / 1024 / 1024);
+ return -1;
+ }
+ speed = bandwidth * 1024ULL * 1024ULL;
if (mon->json)
- ret = qemuMonitorJSONBlockJob(mon, device, base, bandwidth, info, mode,
- async);
+ ret = qemuMonitorJSONBlockJob(mon, device, base, speed, info, mode,
+ modern);
else
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("block jobs require JSON monitor"));
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index f3cdcdd..ffe8fe7 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -527,20 +527,19 @@ int qemuMonitorSendKey(qemuMonitorPtr mon,
unsigned int nkeycodes);
typedef enum {
- BLOCK_JOB_ABORT = 0,
- BLOCK_JOB_INFO = 1,
- BLOCK_JOB_SPEED = 2,
- BLOCK_JOB_SPEED_INTERNAL = 3,
- BLOCK_JOB_PULL = 4,
-} BLOCK_JOB_CMD;
+ BLOCK_JOB_ABORT,
+ BLOCK_JOB_INFO,
+ BLOCK_JOB_SPEED,
+ BLOCK_JOB_PULL,
+} qemuMonitorBlockJobCmd;
int qemuMonitorBlockJob(qemuMonitorPtr mon,
const char *device,
const char *back,
unsigned long bandwidth,
virDomainBlockJobInfoPtr info,
- int mode,
- bool async)
+ qemuMonitorBlockJobCmd mode,
+ bool modern)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuMonitorOpenGraphics(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index eb58f13..e1f5453 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3423,29 +3423,36 @@ static int qemuMonitorJSONGetBlockJobInfo(virJSONValuePtr reply,
}
+/* speed is in bytes/sec */
int
qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
const char *device,
const char *base,
- unsigned long bandwidth,
+ unsigned long long speed,
virDomainBlockJobInfoPtr info,
- int mode,
- bool async)
+ qemuMonitorBlockJobCmd mode,
+ bool modern)
{
int ret = -1;
virJSONValuePtr cmd = NULL;
virJSONValuePtr reply = NULL;
const char *cmd_name = NULL;
- if (base && (mode != BLOCK_JOB_PULL || !async)) {
+ if (base && (mode != BLOCK_JOB_PULL || !modern)) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("only modern block pull supports base: %s"), base);
return -1;
}
+ if (speed && mode == BLOCK_JOB_PULL && !modern) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("only modern block pull supports speed: %llu"),
+ speed);
+ return -1;
+ }
- switch ((BLOCK_JOB_CMD) mode) {
+ switch (mode) {
case BLOCK_JOB_ABORT:
- cmd_name = async ? "block-job-cancel" : "block_job_cancel";
+ cmd_name = modern ? "block-job-cancel" : "block_job_cancel";
cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", device, NULL);
break;
case BLOCK_JOB_INFO:
@@ -3453,19 +3460,24 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL);
break;
case BLOCK_JOB_SPEED:
- case BLOCK_JOB_SPEED_INTERNAL:
- cmd_name = async ? "block-job-set-speed" : "block_job_set_speed";
- cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device",
- device, "U:value",
- bandwidth * 1024ULL * 1024ULL,
- NULL);
+ cmd_name = modern ? "block-job-set-speed" : "block_job_set_speed";
+ cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", device,
+ modern ? "U:speed" : "U:value",
+ speed, NULL);
break;
case BLOCK_JOB_PULL:
- cmd_name = async ? "block-stream" : "block_stream";
- cmd = qemuMonitorJSONMakeCommand(cmd_name,
- "s:device", device,
- base ? "s:base" : NULL, base,
- NULL);
+ cmd_name = modern ? "block-stream" : "block_stream";
+ if (speed)
+ cmd = qemuMonitorJSONMakeCommand(cmd_name,
+ "s:device", device,
+ "U:speed", speed,
+ base ? "s:base" : NULL, base,
+ NULL);
+ else
+ cmd = qemuMonitorJSONMakeCommand(cmd_name,
+ "s:device", device,
+ base ? "s:base" : NULL, base,
+ NULL);
break;
}
@@ -3477,14 +3489,9 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
if (ret == 0 && virJSONValueObjectHasKey(reply, "error")) {
ret = -1;
if (qemuMonitorJSONHasError(reply, "DeviceNotActive")) {
- /* If a job completes before we get a chance to set the
- * speed, we don't want to fail the original command. */
- if (mode == BLOCK_JOB_SPEED_INTERNAL)
- ret = 0;
- else
- qemuReportError(VIR_ERR_OPERATION_INVALID,
- _("No active operation on device: %s"),
- device);
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
+ _("No active operation on device: %s"),
+ device);
} else if (qemuMonitorJSONHasError(reply, "DeviceInUse")){
qemuReportError(VIR_ERR_OPERATION_FAILED,
_("Device %s in use"), device);
@@ -3497,7 +3504,7 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
_("Command '%s' is not found"), cmd_name);
} else {
qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unexpected error"));
+ _("Unexpected error"));
}
}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index aacbb5f..22a3adf 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -251,10 +251,10 @@ int qemuMonitorJSONScreendump(qemuMonitorPtr mon,
int qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
const char *device,
const char *base,
- unsigned long bandwidth,
+ unsigned long long speed,
virDomainBlockJobInfoPtr info,
- int mode,
- bool async)
+ qemuMonitorBlockJobCmd mode,
+ bool modern)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuMonitorJSONSetLink(qemuMonitorPtr mon,
--
1.7.7.6
13 years
[libvirt] [PATCH] python: Fix doc directory name for stable releases
by Cole Robinson
We were using the libvirt release version (like 0.9.11) and not
the configure version (which for stable releases is 0.9.11.X)
Most other places got this right so hopefully that's all the fallout
from the version format change :)
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
python/Makefile.am | 2 +-
python/tests/Makefile.am | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/python/Makefile.am b/python/Makefile.am
index 0305bcc..02b59eb 100644
--- a/python/Makefile.am
+++ b/python/Makefile.am
@@ -18,7 +18,7 @@ INCLUDES = \
AM_CFLAGS = $(WARN_CFLAGS)
-DOCS_DIR = $(datadir)/doc/libvirt-python-$(LIBVIRT_VERSION)
+DOCS_DIR = $(datadir)/doc/libvirt-python-$(VERSION)
DOCS = ${srcdir}/TODO
diff --git a/python/tests/Makefile.am b/python/tests/Makefile.am
index 2a5bc62..c387825 100644
--- a/python/tests/Makefile.am
+++ b/python/tests/Makefile.am
@@ -1,7 +1,7 @@
## Copyright (C) 2005-2011 Red Hat, Inc.
## See COPYING.LIB for the License of this software
-EXAMPLE_DIR = $(datadir)/doc/libvirt-python-$(LIBVIRT_VERSION)/examples
+EXAMPLE_DIR = $(datadir)/doc/libvirt-python-$(VERSION)/examples
PYTESTS= \
basic.py \
--
1.7.7.6
13 years
[libvirt] [PATCH] configure: Use ustar format for dist tarball
by Cole Robinson
Since for stable releases, some test files were over the 99 char
limit for traditional tar filenames.
Suggested by Osier here:
https://www.redhat.com/archives/libvir-list/2012-April/msg01435.html
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
configure.ac | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/configure.ac b/configure.ac
index 89fe818..a819898 100644
--- a/configure.ac
+++ b/configure.ac
@@ -9,7 +9,7 @@ AC_CONFIG_AUX_DIR([build-aux])
AC_CONFIG_HEADERS([config.h])
AC_CONFIG_MACRO_DIR([m4])
dnl Make automake keep quiet about wildcards & other GNUmake-isms
-AM_INIT_AUTOMAKE([-Wno-portability])
+AM_INIT_AUTOMAKE([-Wno-portability tar-ustar])
AM_MAINTAINER_MODE([enable])
# Maintainer note - comment this line out if you plan to rerun
--
1.7.7.6
13 years
[libvirt] [ANNOUNCE] libvirt-glib 0.0.8 release
by Daniel P. Berrange
am pleased to announce that a new release of the libvirt-glib package,
version 0.0.8 is now available from
ftp://libvirt.org/libvirt/glib/
The packages are GPG signed with
Key fingerprint: DAF3 A6FD B26B 6291 2D0E 8E3F BE86 EBB4 1510 4FDF (4096R)
New in this release:
- Fix build of test suites with Debian's libtool
- Add API for disk source startup policy
- Add API for domain controller devices
- Add APIs for USB controllers
- Add APIs for USB / PCI device addressing schemes
- Add APIs for USB redirection devices
- Add ASync API for fetching domain info
- Add APIs for domain timer configuration
libvirt-glib comprises three distinct libraries:
- libvirt-glib - Integrate with the GLib event loop and error handling
- libvirt-gconfig - Representation of libvirt XML documents as GObjects
- libvirt-gobject - Mapping of libvirt APIs into the GObject type system
NB: While libvirt aims to be API/ABI stable, for the first few releases,
we are *NOT* guaranteeing that libvirt-glib libraries are API/ABI stable.
ABI stability will only be guaranteed once the bulk of the APIs have been
fleshed out and proved in non-trivial application usage. We anticipate
this will be within the next 6 months in order to line up with Fedora 17.
Follow up comments about libvirt-glib should be directed to the regular
libvir-list redhat com development list.
Thanks to all the people involved in contributing to this release.
--
|: 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 :|
13 years
[libvirt] [PATCH] lxc: Fix coverity findings
by Stefan Berger
Fix lxc related coverity findings...
Error: UNINIT:
/libvirt/src/lxc/lxc_driver.c:1412:
var_decl: Declaring variable "fd" without initializer.
/libvirt/src/lxc/lxc_driver.c:1460:
uninit_use_in_call: Using uninitialized value "fd" when calling
"virFileClose".
/libvirt/src/util/virfile.c:50:
read_parm: Reading a parameter value.
Error: DEADCODE:
/libvirt/src/lxc/lxc_controller.c:960:
dead_error_condition: On this path, the condition "ret == 4" cannot be true.
/libvirt/src/lxc/lxc_controller.c:959:
at_most: After this line, the value of "ret" is at most -1.
/libvirt/src/lxc/lxc_controller.c:959:
new_values: Noticing condition "ret < 0".
/libvirt/src/lxc/lxc_controller.c:961:
dead_error_line: Execution cannot reach this statement "continue;".
Error: UNINIT:
/libvirt/src/lxc/lxc_controller.c:1104:
var_decl: Declaring variable "consoles" without initializer.
/libvirt/src/lxc/lxc_controller.c:1237:
uninit_use: Using uninitialized value "consoles".
---
src/lxc/lxc_controller.c | 4 ++--
src/lxc/lxc_driver.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
Index: libvirt-acl/src/lxc/lxc_driver.c
===================================================================
--- libvirt-acl.orig/src/lxc/lxc_driver.c
+++ libvirt-acl/src/lxc/lxc_driver.c
@@ -1409,7 +1409,7 @@ static int lxcMonitorClient(lxc_driver_t
virDomainObjPtr vm)
{
char *sockpath = NULL;
- int fd;
+ int fd = -1;
struct sockaddr_un addr;
if (virAsprintf(&sockpath, "%s/%s.sock",
Index: libvirt-acl/src/lxc/lxc_controller.c
===================================================================
--- libvirt-acl.orig/src/lxc/lxc_controller.c
+++ libvirt-acl/src/lxc/lxc_controller.c
@@ -957,7 +957,7 @@ static void lxcEpollIO(int watch, int fd
int ret;
ret = epoll_wait(console->epollFd, &event, 1, 0);
if (ret < 0) {
- if (ret == EINTR)
+ if (errno == EINTR)
continue;
virReportSystemError(errno, "%s",
_("Unable to wait on epoll"));
@@ -1101,7 +1101,7 @@ static int lxcControllerMain(int serverF
size_t nFds,
pid_t container)
{
- struct lxcConsole *consoles;
+ struct lxcConsole *consoles = NULL;
struct lxcMonitor monitor = {
.serverFd = serverFd,
.clientFd = clientFd,
13 years
[libvirt] Memory leak due to virCopyError()
by Stefan Berger
If someone has the time ... I am seeing a memory leak in this code path.
The leak seems to be triggerable by shutting down a VM:
==4717== 40 bytes in 1 blocks are definitely lost in loss record 547 of
1,014
==4717== at 0x4A05E46: malloc (vg_replace_malloc.c:195)
==4717== by 0x38EC27FC01: strdup (strdup.c:43)
==4717== by 0x4EA55E0: virCopyError (virterror.c:255)
==4717== by 0x4EA58B1: virCopyLastError (virterror.c:356)
==4717== by 0x4CB4B6: qemuMonitorIO (qemu_monitor.c:646)
==4717== by 0x4E80C7F: virEventPollDispatchHandles (event_poll.c:490)
==4717== by 0x4E8150B: virEventPollRunOnce (event_poll.c:637)
==4717== by 0x4E7F5B7: virEventRunDefaultImpl (event.c:247)
==4717== by 0x4FA4F50: virNetServerRun (virnetserver.c:713)
==4717== by 0x4221AF: main (libvirtd.c:1133)
My guess is the leak is related to the initialization of the error
structure as for example here:
int
virConnCopyLastError(virConnectPtr conn, virErrorPtr to)
{
/* We can't guarantee caller has initialized it to zero */
memset(to, 0, sizeof(*to));
if (conn == NULL)
return -1;
virMutexLock(&conn->lock);
if (conn->err.code == VIR_ERR_OK)
virResetError(to);
else
After memset'ting 'to' to all zeros, the call to virResetError() seems
pointless... Such memsets are also in other places potentially
overwriting previously allocated pointers...
Stefan
13 years
[libvirt] [libvirt-TCK][PATCH] use 'raw' format as the format of backing file of qcow2 image
by Guannan Ren
If we don't explicitly specify the format of backing file,
it should use raw by default, if so, libvirt's security drivers
should *not* grant access to the last.img file & the guest
should not see the last.img data. That is the purpose of testing.
---
scripts/qemu/205-qcow2-double-backing-file.t | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/scripts/qemu/205-qcow2-double-backing-file.t b/scripts/qemu/205-qcow2-double-backing-file.t
index d3a5e33..ad54c7c 100644
--- a/scripts/qemu/205-qcow2-double-backing-file.t
+++ b/scripts/qemu/205-qcow2-double-backing-file.t
@@ -114,7 +114,6 @@ SKIP: {
my $volmainxml = $tck->generic_volume("tck-main", "qcow2", 1024*1024*50)
->backing_file($pathback)
- ->backing_format("qcow2")
->allocation(0)->as_xml;
--
1.7.7.5
13 years
[libvirt] [PATCH 2/2] [TCK] nwfilter: Add test cases for ipset
by Stefan Berger
Add test cases for the ipset extension.
Since ipset may not be available on all system, the first line of the XML
file containing the test filter has been extended with a specially formatted
XML comment containing a command line test for whether the test case can be
run at all. The format of that line is:
<!-- #<command line test># -->
If the tests in this line don't succeed, the test case is skipped.
Also add a test case cleaning up the created ipset. Run this test after all
other tests using alphabetical ordering.
---
scripts/nwfilter/nwfilter2vmtest.sh | 36 +++++++--
scripts/nwfilter/nwfilterxml2fwallout/ipset-test.fwall | 39 ++++++++++
scripts/nwfilter/nwfilterxml2fwallout/zzz-ipset-cleanup.fwall | 1
scripts/nwfilter/nwfilterxml2xmlin/ipset-test.xml | 25 ++++++
scripts/nwfilter/nwfilterxml2xmlin/zzz-ipset-cleanup.xml | 5 +
5 files changed, 99 insertions(+), 7 deletions(-)
Index: libvirt-tck/scripts/nwfilter/nwfilterxml2xmlin/ipset-test.xml
===================================================================
--- /dev/null
+++ libvirt-tck/scripts/nwfilter/nwfilterxml2xmlin/ipset-test.xml
@@ -0,0 +1,25 @@
+<!-- #ipset help && iptables -t match-set -h && ipset list tck_test || ipset create tck_test hash:ip# -->
+<filter name='tck-testcase' chain='root'>
+ <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid>
+ <rule action='accept' direction='out'>
+ <all ipset='tck_test' ipsetflags='src,dst' />
+ </rule>
+ <rule action='accept' direction='in'>
+ <all state='NONE' ipset='tck_test' ipsetflags='src,dst' comment='in+NONE'/>
+ </rule>
+ <rule action='accept' direction='out'>
+ <all state='NONE' ipset='tck_test' ipsetflags='src,dst' comment='out+NONE'/>
+ </rule>
+ <rule action='accept' direction='in'>
+ <all ipset='tck_test' ipsetflags='SRC,DST,SRC' />
+ </rule>
+ <rule action='accept' direction='in'>
+ <all ipset='tck_test' ipsetflags='SRC,dSt,SRC' />
+ </rule>
+ <rule action='accept' direction='in'>
+ <all ipset='$IPSETNAME' ipsetflags='src,dst' />
+ </rule>
+ <rule action='accept' direction='inout'>
+ <all ipset='$IPSETNAME' ipsetflags='src,dst' comment='inout'/>
+ </rule>
+</filter>
Index: libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh
===================================================================
--- libvirt-tck.orig/scripts/nwfilter/nwfilter2vmtest.sh
+++ libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh
@@ -107,6 +107,7 @@ checkExpectedOutput() {
ifname="$3"
flags="$4"
skipregex="$5"
+ skiptest="$6"
regex="s/${ORIG_IFNAME}/${ifname}/g"
tmpdir=$(mktmpdir)
@@ -147,6 +148,18 @@ checkExpectedOutput() {
break
fi
+ if [ -n "${skiptest}" ]; then
+ # treat all skips as passes
+ passctr=$(($passctr + 1))
+ [ $(($flags & $FLAG_VERBOSE)) -ne 0 ] && \
+ echo "SKIP ${xmlfile} : ${cmd}"
+ [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \
+ test_result $(($passctr + $failctr)) "" 0
+ [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \
+ tap_pass $(($passctr + $failctr)) "SKIP: ${xmlfile} : ${skiptest}"
+ break
+ fi
+
diff -w ${tmpfile} ${tmpfile2} >/dev/null
if [ $? -ne 0 ]; then
@@ -197,19 +210,27 @@ doTest() {
flags="$5"
testnum="$6"
ctr=0
+ skiptest=""
if [ ! -r "${xmlfile}" ]; then
echo "FAIL : Cannot access filter XML file ${xmlfile}."
return 1
fi
- ${VIRSH} nwfilter-define "${xmlfile}" > /dev/null
+ # Check whether we can run this test at all
+ cmd=`sed -n '1,1 s/^<\!--[ ^I]*#\(.*\)#[ ^I]*-->/\1/p' ${xmlfile}`
+ if [ -n "${cmd}" ]; then
+ eval "${cmd}" 2>&1 1>/dev/null
+ [ $? -ne 0 ] && skiptest="${cmd}"
+ fi
+
+ [ -z "${skiptest}" ] && ${VIRSH} nwfilter-define "${xmlfile}" > /dev/null
checkExpectedOutput "${xmlfile}" "${fwallfile}" "${vm1name}" "${flags}" \
- ""
+ "" "${skiptest}"
checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \
- "${vm2name}" "${flags}" ""
+ "${vm2name}" "${flags}" "" "${skiptest}"
if [ $(($flags & $FLAG_ATTACH)) -ne 0 ]; then
@@ -234,9 +255,9 @@ EOF
if [ $rc -eq 0 ]; then
checkExpectedOutput "${xmlfile}" "${fwallfile}" "${ATTACH_IFNAME}" \
- "${flags}" "(PRE|POST)ROUTING"
+ "${flags}" "(PRE|POST)ROUTING" "${skiptest}"
checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \
- "${vm2name}" "${flags}" "(PRE|POST)ROUTING"
+ "${vm2name}" "${flags}" "(PRE|POST)ROUTING" "${skiptest}"
msg=`${VIRSH} detach-device "${vm1name}" "${tmpfile}"`
if [ $? -ne 0 ]; then
echo "FAIL: Detach of interface failed."
@@ -246,9 +267,9 @@ EOF
# In case of TAP, run the test anyway so we get to the full number
# of tests
checkExpectedOutput "${xmlfile}" "${fwallfile}" "${ATTACH_IFNAME}" \
- "${flags}" "" #"(PRE|POST)ROUTING"
+ "${flags}" "" "${skiptest}" #"(PRE|POST)ROUTING"
checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \
- "${vm2name}" "${flags}" #"(PRE|POST)ROUTING"
+ "${vm2name}" "${flags}" "${skiptest}" #"(PRE|POST)ROUTING"
fi
attachfailctr=$(($attachfailctr + 1))
@@ -357,6 +378,7 @@ createVM() {
<parameter name='C' value='1090'/>
<parameter name='C' value='1100'/>
<parameter name='C' value='1110'/>
+ <parameter name='IPSETNAME' value='tck_test'/>
</filterref>
<target dev='${vmname}'/>
</interface>
Index: libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/ipset-test.fwall
===================================================================
--- /dev/null
+++ libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/ipset-test.fwall
@@ -0,0 +1,39 @@
+#iptables -L FI-vnet0 -n
+Chain FI-vnet0 (1 references)
+target prot opt source destination
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 state NEW,ESTABLISHED ctdir REPLY match-set tck_test src,dst
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 match-set tck_test src,dst /* out+NONE */
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 state ESTABLISHED ctdir ORIGINAL match-set tck_test dst,src,dst
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 state ESTABLISHED ctdir ORIGINAL match-set tck_test dst,src,dst
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 state ESTABLISHED ctdir ORIGINAL match-set tck_test dst,src
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 match-set tck_test dst,src /* inout */
+#iptables -L FO-vnet0 -n
+Chain FO-vnet0 (1 references)
+target prot opt source destination
+ACCEPT all -- 0.0.0.0/0 0.0.0.0/0 state ESTABLISHED ctdir ORIGINAL match-set tck_test dst,src
+ACCEPT all -- 0.0.0.0/0 0.0.0.0/0 match-set tck_test src,dst /* in+NONE */
+ACCEPT all -- 0.0.0.0/0 0.0.0.0/0 state NEW,ESTABLISHED ctdir REPLY match-set tck_test src,dst,src
+ACCEPT all -- 0.0.0.0/0 0.0.0.0/0 state NEW,ESTABLISHED ctdir REPLY match-set tck_test src,dst,src
+ACCEPT all -- 0.0.0.0/0 0.0.0.0/0 state NEW,ESTABLISHED ctdir REPLY match-set tck_test src,dst
+ACCEPT all -- 0.0.0.0/0 0.0.0.0/0 match-set tck_test src,dst /* inout */
+#iptables -L HI-vnet0 -n
+Chain HI-vnet0 (1 references)
+target prot opt source destination
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 state NEW,ESTABLISHED ctdir REPLY match-set tck_test src,dst
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 match-set tck_test src,dst /* out+NONE */
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 state ESTABLISHED ctdir ORIGINAL match-set tck_test dst,src,dst
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 state ESTABLISHED ctdir ORIGINAL match-set tck_test dst,src,dst
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 state ESTABLISHED ctdir ORIGINAL match-set tck_test dst,src
+RETURN all -- 0.0.0.0/0 0.0.0.0/0 match-set tck_test dst,src /* inout */
+#iptables -L libvirt-host-in -n | grep vnet0 | tr -s " "
+HI-vnet0 all -- 0.0.0.0/0 0.0.0.0/0 [goto] PHYSDEV match --physdev-in vnet0
+#iptables -L libvirt-in -n | grep vnet0 | tr -s " "
+FI-vnet0 all -- 0.0.0.0/0 0.0.0.0/0 [goto] PHYSDEV match --physdev-in vnet0
+#iptables -L libvirt-in-post -n | grep vnet0
+ACCEPT all -- 0.0.0.0/0 0.0.0.0/0 PHYSDEV match --physdev-in vnet0
+#iptables -L libvirt-out -n | grep vnet0 | tr -s " "
+FO-vnet0 all -- 0.0.0.0/0 0.0.0.0/0 [goto] PHYSDEV match --physdev-out vnet0
+#ebtables -t nat -L libvirt-O-vnet0 2>/dev/null | grep -v "table:" | grep -v "^$"
+#ebtables -t nat -L libvirt-I-vnet0 2>/dev/null | grep -v "table:" | grep -v "^$"
+#ebtables -t nat -L PREROUTING | grep vnet0
+#ebtables -t nat -L POSTROUTING | grep vnet0
Index: libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/zzz-ipset-cleanup.fwall
===================================================================
--- /dev/null
+++ libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/zzz-ipset-cleanup.fwall
@@ -0,0 +1 @@
+#ipset destroy tck_test 2>&1 1>/dev/null
Index: libvirt-tck/scripts/nwfilter/nwfilterxml2xmlin/zzz-ipset-cleanup.xml
===================================================================
--- /dev/null
+++ libvirt-tck/scripts/nwfilter/nwfilterxml2xmlin/zzz-ipset-cleanup.xml
@@ -0,0 +1,5 @@
+<!-- #ipset help && iptables -t match-set -h# -->
+<filter name='tck-testcase' chain='root'>
+ <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid>
+ <!-- used only to cleanup ipset -->
+</filter>
13 years
