[libvirt] [PATCH 2/2 v2] qemu: Fix SW emulated machines
This patch fixes building a command-line for QEMU machines without KVM
acceleration and is based on following assumptions:
- QEMU_CAPS_KVM flag means that QEMU is capable of running KVM
accelerated machine (not that it knows about KVM at all, even
though there is probably no QEMU that knows about KVM and isn't
able to use it). It is not actually true for the past (it meant
that QEMU has '-no-kvm' option, that it doesn't always know), but
we can say this is what it means from now on without any harm being
done.
- QEMU_CAPS_ENABLE_KVM flag means that QEMU is, by default, running
without KVM acceleration and in case we need KVM acceleration it
needs to be explicitely instructed to do so. This is partially
true for the past (this option essentially means that QEMU
recognizes the '-enable-kvm' option, even though it's almost the
same).
---
src/qemu/qemu_capabilities.c | 16
src/qemu/qemu_command.c | 6 --
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index fe462e9..a19d833 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2039,9 +2039,18 @@ qemuCapsProbeQMPKVMState(qemuCapsPtr caps,
if (qemuMonitorGetKVMState(mon, enabled, present) 0)
return -1;
-/* Youre right, this code does nothing, you must have checked out
- * some weird commit. Go back to your room and think about what
- * you've done, young (wo)man. */
+if (!present) {
+/* When KVM is not present, the flag shouldn't be set, but it
+ * was set earlier when we discovered that QEMU is capable of
+ * query-kvm QMP command (which doesn't mean anything,
+ * essentially). So let's fix that. */
+qemuCapsClear(caps, QEMU_CAPS_KVM);
+} else if (!enabled) {
+/* We shouldn't use -enable-kvm when kvm is enabled by
+ * default, but more importantly, we should be able to know
+ * when not using it launches a software emulated machine. */
+qemuCapsSet(caps, QEMU_CAPS_ENABLE_KVM);
+}
return 0;
}
@@ -2177,7 +2186,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
qemuCapsSet(caps, QEMU_CAPS_DRIVE_SERIAL);
qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_UNIX);
qemuCapsSet(caps, QEMU_CAPS_CHARDEV);
-qemuCapsSet(caps, QEMU_CAPS_ENABLE_KVM);
qemuCapsSet(caps, QEMU_CAPS_MONITOR_JSON);
qemuCapsSet(caps, QEMU_CAPS_BALLOON);
qemuCapsSet(caps, QEMU_CAPS_DEVICE);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 898c4c0..2f863d3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4466,12 +4466,14 @@ qemuBuildCommandLine(virConnectPtr conn,
case VIR_DOMAIN_VIRT_QEMU:
if (qemuCapsGet(caps, QEMU_CAPS_KQEMU))
disableKQEMU = 1;
-if (qemuCapsGet(caps, QEMU_CAPS_KVM))
+if (qemuCapsGet(caps, QEMU_CAPS_KVM)
+!qemuCapsGet(caps, QEMU_CAPS_ENABLE_KVM))
disableKVM = 1;
break;
case VIR_DOMAIN_VIRT_KQEMU:
-if (qemuCapsGet(caps, QEMU_CAPS_KVM))
+if (qemuCapsGet(caps, QEMU_CAPS_KVM)
+!qemuCapsGet(caps, QEMU_CAPS_ENABLE_KVM))
disableKVM = 1;
if (qemuCapsGet(caps, QEMU_CAPS_ENABLE_KQEMU)) {
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2 v1] qemu: Fix SW emulated machines
This patch fixes building a command-line for QEMU machines without KVM
acceleration and is based on following assumptions:
- QEMU_CAPS_KVM flag means that QEMU is running KVM accelerated
machines by default (without explicitely requesting that using a
command-line option). It is the closest to the true according to
the code with the only exception being the comment next to the
flag, so it's fixed in this patch as well.
- QEMU_CAPS_ENABLE_KVM flag means that QEMU is, by default, running
without KVM acceleration and in case we need KVM acceleration it
needs to be explicitely instructed to do so. This is partially
true for the past (this option essentially means that QEMU
recognizes the '-enable-kvm' option, even though it's almost the
same).
---
src/qemu/qemu_capabilities.c | 13 +
src/qemu/qemu_capabilities.h | 2 +-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index fe462e9..aad366d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2039,9 +2039,15 @@ qemuCapsProbeQMPKVMState(qemuCapsPtr caps,
if (qemuMonitorGetKVMState(mon, enabled, present) 0)
return -1;
-/* Youre right, this code does nothing, you must have checked out
- * some weird commit. Go back to your room and think about what
- * you've done, young (wo)man. */
+/* Until now, the QEMU_CAPS_KVM was set according to the QEMU
+ * reporting the recongnition of 'query-kvm' QMP command, but the
+ * flag means whether the KVM is enabled by default and should be
+ * disabled in case we want SW emulated machine, so let's fix that
+ * if it's true. */
+if (!enabled) {
+qemuCapsClear(caps, QEMU_CAPS_KVM);
+qemuCapsSet(caps, QEMU_CAPS_ENABLE_KVM);
+}
return 0;
}
@@ -2177,7 +2183,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
qemuCapsSet(caps, QEMU_CAPS_DRIVE_SERIAL);
qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_UNIX);
qemuCapsSet(caps, QEMU_CAPS_CHARDEV);
-qemuCapsSet(caps, QEMU_CAPS_ENABLE_KVM);
qemuCapsSet(caps, QEMU_CAPS_MONITOR_JSON);
qemuCapsSet(caps, QEMU_CAPS_BALLOON);
qemuCapsSet(caps, QEMU_CAPS_DEVICE);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 988dfdb..9c5fd0f 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -45,7 +45,7 @@ enum qemuCapsFlags {
QEMU_CAPS_MIGRATE_QEMU_TCP = 10, /* have qemu tcp migration */
QEMU_CAPS_MIGRATE_QEMU_EXEC = 11, /* have qemu exec migration */
QEMU_CAPS_DRIVE_CACHE_V2 = 12, /* cache= flag wanting new v2 values */
-QEMU_CAPS_KVM= 13, /* Whether KVM is compiled in */
+QEMU_CAPS_KVM= 13, /* Whether KVM is enabled by default */
QEMU_CAPS_DRIVE_FORMAT = 14, /* Is -drive format= avail */
QEMU_CAPS_VGA= 15, /* Is -vga avail */
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] gitignore: ignore more files
On 10/29/2012 07:07 AM, liguang wrote: ignore *.patch, cscope.po.out, cscope.in.out Signed-off-by: liguang lig.f...@cn.fujitsu.com --- .gitignore |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index 1cd2d45..d2d6657 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,7 @@ *.pyc *.rej *.s +*.patch It shouldn't spoil the repo even if we have *.patch files in already (in docs/api_extension/), but doesn't feel clean since there are such files. Anyone feels bad about changing this to '/*.patch' or adding '!/docs/api_extension/*.patch' to this? *~ .deps .gdb_history @@ -185,6 +186,8 @@ TAGS coverage cscope.files cscope.out +cscope.in.out +cscope.po.out results.log stamp-h stamp-h.in This one is ok. BTW: Why don't we have 'tags' in .gitignore as well (created by ctags target)? Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] xml: print uuids or shell-escaped names in the warning
On 10/29/2012 10:38 AM, Jiri Denemark wrote: On Thu, Oct 25, 2012 at 16:37:37 +0200, Ján Tomko wrote: In the XML warning, this prints uuids for domain names with special characters in them and shell-escaped names for other elements (like snapshosts and networks) with special names. --- When saving snapshots, the domain name is appended to the snapshot-edit command, so using a domain name that needs escaping would lead to something that can't be just fed to the shell as it would glue them together. diff to xml: shell-escape domain name in comment: - Domain names don't get escaped, UUIDs are preferred. - The command gets escaped too. diff to v1: don't try to use CDATA (it doesn't belong there) --- src/conf/domain_conf.c |6 +++- src/libvirt_private.syms |2 + src/qemu/qemu_domain.c |3 +- src/util/buf.c | 66 ++ src/util/buf.h |1 + src/util/xml.c | 62 ++- src/util/xml.h |1 + 7 files changed, 109 insertions(+), 32 deletions(-) I think we're making this too complicated for no real benefit. The goal is to provide a hint to anyone who looks at the autogenerated XML files and IMHO providing an escaped string (that would only work in environments for which it was designed). I would just keep it simple: - emit virsh command name in case name is nice - emit virsh command uuid in case name is ugly and uuid is known - emit virsh command in all other cases This should keep the hints in domain and network XML files in /etc/libvirt usable for copypaste (the UUID fallback works there). Snapshot files (located in /var/lib/libvirt) use virsh snapshot-edit domain snapshot, where domains are passed as part of the command and snapshots have no UUIDs. Thus to keep the code simple, I'd emit just virsh snapshot-edit, which is still a useful hint and I don't believe we need to do anything beyond that. Since the comment is merely a warning for people that aren't used to the commands or don't know how libvirt works, I second that opinion. I myself believe there is no need for the whole command anyway, especially when getting to know how to specify the right command-line encourages the user to get to know virsh better. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Fix connection to ESX 5.1
After separating 5.x and 5.1 versions of ESX, we forgot to add 5.1
into the list of allowed connections, so connections to 5.1 fail since
v1.0.0-rc1-5-g1e7cd39
---
src/esx/esx_driver.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 8d13829..2aa6978 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -718,6 +718,7 @@ esxConnectToHost(virConnectPtr conn,
priv-host-productVersion != esxVI_ProductVersion_ESX41
priv-host-productVersion != esxVI_ProductVersion_ESX4x
priv-host-productVersion != esxVI_ProductVersion_ESX50
+priv-host-productVersion != esxVI_ProductVersion_ESX51
priv-host-productVersion != esxVI_ProductVersion_ESX5x) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(%s is neither an ESX 3.5, 4.x nor 5.x host),
@@ -847,6 +848,7 @@ esxConnectToVCenter(virConnectPtr conn,
priv-vCenter-productVersion != esxVI_ProductVersion_VPX41
priv-vCenter-productVersion != esxVI_ProductVersion_VPX4x
priv-vCenter-productVersion != esxVI_ProductVersion_VPX50
+priv-vCenter-productVersion != esxVI_ProductVersion_VPX51
priv-vCenter-productVersion != esxVI_ProductVersion_VPX5x) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(%s is neither a vCenter 2.5, 4.x nor 5.x server),
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Fix connection to ESX 5.1
On 10/30/2012 08:49 AM, Michal Privoznik wrote:
On 30.10.2012 08:35, Martin Kletzander wrote:
After separating 5.x and 5.1 versions of ESX, we forgot to add 5.1
into the list of allowed connections, so connections to 5.1 fail since
v1.0.0-rc1-5-g1e7cd39
---
src/esx/esx_driver.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 8d13829..2aa6978 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -718,6 +718,7 @@ esxConnectToHost(virConnectPtr conn,
priv-host-productVersion != esxVI_ProductVersion_ESX41
priv-host-productVersion != esxVI_ProductVersion_ESX4x
priv-host-productVersion != esxVI_ProductVersion_ESX50
+priv-host-productVersion != esxVI_ProductVersion_ESX51
priv-host-productVersion != esxVI_ProductVersion_ESX5x) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(%s is neither an ESX 3.5, 4.x nor 5.x host),
@@ -847,6 +848,7 @@ esxConnectToVCenter(virConnectPtr conn,
priv-vCenter-productVersion != esxVI_ProductVersion_VPX41
priv-vCenter-productVersion != esxVI_ProductVersion_VPX4x
priv-vCenter-productVersion != esxVI_ProductVersion_VPX50
+priv-vCenter-productVersion != esxVI_ProductVersion_VPX51
priv-vCenter-productVersion != esxVI_ProductVersion_VPX5x) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(%s is neither a vCenter 2.5, 4.x nor 5.x server),
ACK
Michal
Thanks, pushed.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] sanlock: Introduce 'user' and 'group' conf variables
On 10/29/2012 04:18 PM, Michal Privoznik wrote:
through which user set under what permissions does sanlock
daemon run so libvirt will set the same permissions for
files exposed to it.
---
diff to v1:
-update spec file so sanlock dir is installed with root:sanlock
iff group sanlock exists
docs/locking.html.in| 22 +
libvirt.spec.in |7 +++
src/locking/libvirt_sanlock.aug |2 +
src/locking/lock_driver_sanlock.c | 76
++-
src/locking/sanlock.conf| 11 -
src/locking/test_libvirt_sanlock.aug.in |2 +
6 files changed, 118 insertions(+), 2 deletions(-)
diff --git a/docs/locking.html.in b/docs/locking.html.in
index 6d7b517..19dd6a3 100644
--- a/docs/locking.html.in
+++ b/docs/locking.html.in
@@ -121,6 +121,28 @@
/pre
p
+ If your sanlock daemon happen to run under non-root
+ privileges, you need to tell this to libvirt so it
+ chowns created files correctly. This can be done by
+ setting codeuser/code and/or codegroup/code
+ variables in the configuration file. Accepted values
+ range is specified in description to the same
+ variables in code/etc/libvirt/qemu.conf/code. For
+ example:
+/p
+
+pre
+ augtool -s set /files/etc/libvirt/qemu-sanlock.conf/user sanlock
+ augtool -s set /files/etc/libvirt/qemu-sanlock.conf/group sanlock
+/pre
+
+p
+ But remember, that if this is NFS share, you need a
+ no_root_squash-ed one for chown (and chmod possibly)
+ to succeed.
+/p
+
+p
In terms of storage requirements, if the filesystem
uses 512 byte sectors, you need to allow for code1MB/code
of storage for each guest disk. So if you have a network
diff --git a/libvirt.spec.in b/libvirt.spec.in
index ebebfab..edc43af 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1568,6 +1568,13 @@ fi
/bin/systemctl try-restart libvirt-guests.service /dev/null 21 || :
%endif
+%pre lock-sanlock
+if $(getent group sanlock /dev/null; echo $?) == 0
+chmod 0770 %{_localstatedir}/lib/libvirt/sanlock
+chown root:sanlock %{_localstatedir}/lib/libvirt/sanlock
+endif
Change this to:
%post lock-sanlock
if getent group sanlock /dev/null; then
chmod 0770 %{_localstatedir}/lib/libvirt/sanlock
chown root:sanlock %{_localstatedir}/lib/libvirt/sanlock
fi
and you've got my ACK (we should make this working in 1.0.0,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Enhance QMP detection of KVM state
On 10/31/2012 12:44 AM, Eric Blake wrote:
On 10/28/2012 06:55 AM, Martin Kletzander wrote:
When there is no 'qemu-kvm' binary and the emulator used for a machine
is, for example, 'qemu-system-x86_64' that, by default, runs without
kvm enabled, libvirt still supplies '-no-kvm' option to this process,
even though it does not recognize such option (making the start of a
domain fail in that case).
This patch adds QMP querying for KVM state using 'query-kvm' state,
but does not set any of QEMU_CAPS_KVM and QEMU_CAPS_ENABLE_KVM flags.
That functionality is done in different patch in order to be able to
compare two possibilities and chose the better one without looking at
the part of the code that's exactly the same for both of them (this
patch).
+static int
+qemuCapsProbeQMPKVMState(qemuCapsPtr caps,
+ qemuMonitorPtr mon)
+{
+bool enabled = false;
+bool present = false;
+
+if (!qemuCapsGet(caps, QEMU_CAPS_KVM))
+return 0;
+
+if (qemuMonitorGetKVMState(mon, enabled, present) 0)
+return -1;
+
+/* Youre right, this code does nothing, you must have checked out
+ * some weird commit. Go back to your room and think about what
+ * you've done, young (wo)man. */
Since this cute comment disappears in either 2/2 approach, I would
probably rather see this series squashed into a single commit when
finally going upstream. That would also mean deleting the second
paragraph of the commit message.
+int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon,
+ bool *enabled,
+ bool *present)
+{
+int ret;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand(query-kvm, NULL)))
+return -1;
+
+ret = qemuMonitorJSONCommand(mon, cmd, reply);
+
+if (ret == 0) {
+if (qemuMonitorJSONHasError(reply, CommandNotFound))
+goto cleanup;
This relies on the caller to pre-initialize *enabled and *present to
false; it might be better to explicitly repeat that setting here so that
this function guarantees that the values are always correct on
successful return even if the caller forgot to initialize. But right
now, the only caller happens to pre-initialize, so it's not a show-stopper.
ACK, once I pick which of the 2/2 variants I like best :)
I squashed both in, fixed the huge amount of typos, added the explicit
setting to false for the bools in qemuMonitorJSONGetKVMState(), double
checked and pushed. Thanks very much.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Improve error reporting from absolutePathFromBaseFile helper
On 10/31/2012 11:19 AM, Peter Krempa wrote:
There are multiple reasons canonicalize_file_name() used in
absolutePathFromBaseFile helper can fail. This patch enhances error
reporting from that helper.
---
src/util/storage_file.c | 35 ---
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index e0b4178..f4c2943 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -530,22 +530,36 @@ qedGetBackingStore(char **res,
static char *
absolutePathFromBaseFile(const char *base_file, const char *path)
{
-char *res;
-char *tmp;
-size_t d_len = dir_len (base_file);
+char *res = NULL;
+char *tmp = NULL;
+size_t d_len = dir_len(base_file);
/* If path is already absolute, or if dirname(base_file) is .,
just return a copy of path. */
-if (*path == '/' || d_len == 0)
-return canonicalize_file_name(path);
+if (*path == '/' || d_len == 0) {
+if (!(res = canonicalize_file_name(path)))
+virReportSystemError(errno,
+ _(Can't canonicalize path '%s'), path);
+
+goto cleanup;
+}
/* Ensure that the following cast-to-int is valid. */
-if (d_len INT_MAX)
-return NULL;
+if (d_len INT_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Directory name too long: '%s', base_file);
Forgot to gettext here: _(Directory name too long: '%s')
+goto cleanup;
+}
-if (virAsprintf(tmp, %.*s/%s, (int) d_len, base_file, path) 0)
-return NULL;
-res = canonicalize_file_name(tmp);
+if (virAsprintf(tmp, %.*s/%s, (int) d_len, base_file, path) 0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (!(res = canonicalize_file_name(tmp)))
+virReportSystemError(errno, _(Can't canonicalize path '%s'), path);
+
+cleanup:
VIR_FREE(tmp);
return res;
}
@@ -713,7 +727,6 @@ virStorageFileGetMetadataFromBuf(int format,
meta-backingStoreRaw = meta-backingStore;
meta-backingStore = absolutePathFromBaseFile(path, backing);
if (meta-backingStore == NULL) {
-virReportOOMError();
VIR_FREE(backing);
return -1;
}
ACK with that fixed,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Availability of Release Candidate 3 for 1.0.0
On 10/31/2012 01:29 PM, Leonardo Arena wrote: Hi, I was invited to show up here regarding BZ 871756. I'm the maintainer of libvirt on Alpine Linux [1], an uclibc-based distro. Hi, thanks for looking at it. It's good to hear there is only one function missing when building on uclibc. Although it's a pity there is no replacement for that in uclibc, there is in gnulib (which we use, anyway) and I think that adding a gnulib module would be safer, but probably I'm not the best one to decide upon that. Anyone else to correct me? In case anyone wants to have a look, here are the links to ease the search: Bug: https://bugzilla.redhat.com/show_bug.cgi?id=871756 Diff: https://bugzilla.redhat.com/attachment.cgi?id=636010 If you need more info, testing, etc. just let me know. If you could try that with the gnulib module and send it as a patch, we could maybe have it in 1.0.0. Martin Cheers, leonardo P.S. Sorry if I've broken the thread [1] http://alpinelinux.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix EmulatorPinInfo without emulatorpin
https://bugzilla.redhat.com/show_bug.cgi?id=871312
Recent fixes made almost all the right steps to make emulator pinned
to the cpuset of the whole domain in case emulatorpin isn't
specified, but qemudDomainGetEmulatorPinInfo still reports all the
CPUs even when cpuset is specified. This patch fixes that.
---
src/qemu/qemu_driver.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3980c10..8b5f06a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4352,7 +4352,6 @@ qemudDomainGetEmulatorPinInfo(virDomainPtr dom,
virDomainDefPtr targetDef = NULL;
int ret = -1;
int maxcpu, hostcpus, pcpu;
-virDomainVcpuPinDefPtr emulatorpin = NULL;
virBitmapPtr cpumask = NULL;
bool pinned;
@@ -4394,14 +4393,15 @@ qemudDomainGetEmulatorPinInfo(virDomainPtr dom,
cpumaps[maplen - 1] = (1 maxcpu % 8) - 1;
}
-/* If no emulatorpin, all cpus should be used */
-emulatorpin = targetDef-cputune.emulatorpin;
-if (!emulatorpin) {
+if (targetDef-cputune.emulatorpin) {
+cpumask = targetDef-cputune.emulatorpin-cpumask;
+} else if (targetDef-cpumask) {
+cpumask = targetDef-cpumask;
+} else {
ret = 0;
goto cleanup;
}
-cpumask = emulatorpin-cpumask;
for (pcpu = 0; pcpu maxcpu; pcpu++) {
if (virBitmapGetBit(cpumask, pcpu, pinned) 0)
goto cleanup;
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix EmulatorPinInfo without emulatorpin
On 10/31/2012 04:11 PM, Jiri Denemark wrote: On Wed, Oct 31, 2012 at 16:03:53 +0100, Martin Kletzander wrote: https://bugzilla.redhat.com/show_bug.cgi?id=871312 Recent fixes made almost all the right steps to make emulator pinned to the cpuset of the whole domain in case emulatorpin isn't specified, but qemudDomainGetEmulatorPinInfo still reports all the CPUs even when cpuset is specified. This patch fixes that. --- src/qemu/qemu_driver.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) ACK Jirka Thanks, pushed. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: prefer mkostemp for multi-thread safety
On 10/31/2012 03:42 PM, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=871756
Commit cd1e8d1 assumed that systems new enough to have journald
also have mkostemp; but this is not true for uclibc.
For that matter, use of mkstemp[s] is unsafe in a multi-threaded
program. We should prefer mkostemp[s] in the first place.
* bootstrap.conf (gnulib_modules): Add mkostemp, mkostemps; drop
mkstemp and mkstemps.
* cfg.mk (sc_prohibit_mkstemp): New syntax check.
* tools/virsh.c (vshEditWriteToTempFile): Adjust caller.
* src/qemu/qemu_driver.c (qemuDomainScreenshot)
(qemudDomainMemoryPeek): Likewise.
* src/secret/secret_driver.c (replaceFile): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainScreenshot): Likewise.
---
bootstrap.conf | 4 ++--
cfg.mk | 6 ++
src/qemu/qemu_driver.c | 8
src/secret/secret_driver.c | 4 ++--
src/vbox/vbox_tmpl.c | 4 ++--
tools/virsh.c | 2 +-
6 files changed, 17 insertions(+), 11 deletions(-)
[...]
diff --git a/tools/virsh.c b/tools/virsh.c
index f0ec625..5388c9e 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -565,7 +565,7 @@ vshEditWriteToTempFile(vshControl *ctl, const char *doc)
vshError(ctl, %s, _(out of memory));
return NULL;
}
-fd = mkstemps(ret, 4);
+fd = mkostemps(ret, 4, O_CLOEXEC);
if (fd == -1) {
vshError(ctl, _(mkstemps: failed to create temporary file: %s),
This message should be changed as well.
virStrerror(errno, ebuf, sizeof(ebuf)));
ACK with that changed.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] gitignore: Ignore 'tags'
On 10/31/2012 04:48 PM, Michal Privoznik wrote: --- .gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index 98ce398..79a055b 100644 --- a/.gitignore +++ b/.gitignore @@ -193,6 +193,7 @@ results.log stamp-h stamp-h.in stamp-h1 +tags !/gnulib/lib/Makefile.am !/gnulib/tests/Makefile.am !/m4/virt-*.m4 ACK, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: libvirtd no longer uses abstract namespace
On 10/31/2012 04:48 PM, Eric Blake wrote: Commit 905be03d2 quit using the abstract namespace, but didn't update the --help text to match. * daemon/libvirtd.c (daemonUsage): Correct socket listing. --- See also: https://www.redhat.com/archives/libvirt-users/2012-October/msg00152.html daemon/libvirtd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index d5f3e4c..624831a 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -936,7 +936,7 @@ libvirt management daemon:\n), argv0); $XDG_CONFIG_HOME/libvirt/libvirtd.conf\n\ \n\ Sockets:\n\ - $XDG_RUNTIME_DIR/libvirt/libvirt-sock (in UNIX abstract namespace)\n\ + $XDG_RUNTIME_DIR/libvirt/libvirt-sock\n\ \n\ TLS:\n\ CA certificate: $HOME/.pki/libvirt/cacert.pem\n\ That's right, I didn't realize we still have it there when I was doing something with it some time ago. ACK, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: prefer mkostemp for multi-thread safety
On 11/01/2012 04:53 PM, Leonardo Arena wrote:
On Thu, 2012-11-01 at 14:51 +0100, Martin Kletzander wrote:
On 11/01/2012 02:32 PM, Leonardo Arena wrote:
On Wed, 2012-10-31 at 10:10 -0600, Eric Blake wrote:
On 10/31/2012 09:45 AM, Martin Kletzander wrote:
On 10/31/2012 03:42 PM, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=871756
Commit cd1e8d1 assumed that systems new enough to have journald
also have mkostemp; but this is not true for uclibc.
For that matter, use of mkstemp[s] is unsafe in a multi-threaded
program. We should prefer mkostemp[s] in the first place.
-fd = mkstemps(ret, 4);
+fd = mkostemps(ret, 4, O_CLOEXEC);
if (fd == -1) {
vshError(ctl, _(mkstemps: failed to create temporary file:
%s),
This message should be changed as well.
virStrerror(errno, ebuf, sizeof(ebuf)));
ACK with that changed.
Fixed and pushed. Hopefully Leonardo can give libvirt.git a test (since
we probably won't have any more rc builds between now and 1.0.0 on Friday).
Unfortunately I'm having some issues in making the distribution, due to
automake affected by CVE-2012-3386, and bootstrapping in Alpine (with fixed
automake) has some other issues. If there's anyone that can send me a
tarball, I'll test it asap.
Thank you
-leonardo
Feel free to grab this one:
http://people.redhat.com/mkletzan/libvirt-0.10.2.tar.gz
I hope doing 'make dist' was enough =)
Martin
It worked.
I've closed BZ871756.
Thanks to all.
- leonardo
And thanks to you as well. Let us know how the 1.0.0 turns out as it is
already released.
Have a nice day,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] logging: Do not redefine mkostemp on uclibc
On 11/02/2012 12:15 PM, Jiri Denemark wrote: An obsolete version of the patch that fixed building on systems with uclibc (4dbd6e96) was accidentally pushed as part of the release commit (2b435c15). Since this obsolete patch made syntax-check, commit 30b398d5 was pushed to fix that issue. But the real fix is to actually remove the offending code. This patch reverts commit 30b398d5 and the relevant part of 2b435c15. --- cfg.mk | 2 -- src/util/logging.c | 5 - 2 files changed, 7 deletions(-) diff --git a/cfg.mk b/cfg.mk index 963f642..cda04e4 100644 --- a/cfg.mk +++ b/cfg.mk @@ -817,5 +817,3 @@ exclude_file_name_regexp--sc_unmarked_diagnostics = \ ^(docs/apibuild.py|tests/virt-aa-helper-test)$$ exclude_file_name_regexp--sc_size_of_brackets = cfg.mk - -exclude_file_name_regexp--sc_prohibit_mkstemp = ^src/util/logging\.c$$ diff --git a/src/util/logging.c b/src/util/logging.c index 27bd74c..dd43842 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -58,11 +58,6 @@ #define VIR_FROM_THIS VIR_FROM_NONE -#ifdef __UCLIBC__ -/* uclibc does not implement mkostemp GNU extention */ -# define mkostemp(x,y) mkstemp(x) -#endif - VIR_ENUM_DECL(virLogSource) VIR_ENUM_IMPL(virLogSource, VIR_LOG_FROM_LAST, file, ACK, this is the desirable version. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Allow the user to specify vendor and product for disk
On 11/05/2012 08:04 AM, Osier Yang wrote:
QEMU supports to set vendor and product strings for disk since
1.2.0 (only scsi-disk, scsi-hd, scsi-cd support it), this patch
exposes it with new XML elements vendor and product of disk
device.
---
docs/formatdomain.html.in | 10 +
docs/schemas/domaincommon.rng | 10 +
src/conf/domain_conf.c | 30
src/conf/domain_conf.h |2 +
src/qemu/qemu_command.c| 29
.../qemuxml2argv-disk-scsi-disk-vpd.args | 13 +++
.../qemuxml2argv-disk-scsi-disk-vpd.xml| 36
tests/qemuxml2argvtest.c |4 ++
8 files changed, 134 insertions(+), 0 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c8da33d..cc9e871 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1657,6 +1657,16 @@
of 16 hexadecimal digits.
span class='since'Since 0.10.1/span
/dd
+ dtcodevendor/code/dt
+ ddIf present, this element specifies the vendor of a virtual hard
+disk or CD-ROM device. It's a not more than 8 bytes alphanumeric
string.
Last sentence doesn't make sense, I suggest changing it to either: It
can't be longer than 8 alphanumeric characters. or simply Maximum 8
alphanumeric characters.
+span class='since'Since 1.0.1/span
+ /dd
+ dtcodeproduct/code/dt
+ ddIf present, this element specifies the product of a virtual hard
+disk or CD-ROM device. It's a not more than 16 bytes alphanumeric
string.
dtto
+span class='since'Since 1.0.1/span
+ /dd
dtcodehost/code/dt
ddThe codehost/code element has two attributes name and port,
which specify the hostname and the port number. The meaning of this
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 2beb035..ed7d1d0 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -905,6 +905,16 @@
ref name=wwn/
/element
/optional
+ optional
+element name=vendor
+ text/
+/element
+ /optional
+ optional
+element name=product
+ text/
+/element
+ /optional
This is little bit different than the other specifications around in the
code and could be made better, but looking at the schema I've found more
inconsistencies, so it's ok for now. I'll send a cleanup patch later
for these and the others as well.
/interleave
/define
define name=snapshot
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0575fcd..db6608e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -979,6 +979,8 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
VIR_FREE(def-mirror);
VIR_FREE(def-auth.username);
VIR_FREE(def-wwn);
+VIR_FREE(def-vendor);
+VIR_FREE(def-product);
if (def-auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
VIR_FREE(def-auth.secret.usage);
virStorageEncryptionFree(def-encryption);
@@ -3498,6 +3500,8 @@ cleanup:
goto cleanup;
}
+#define VENDOR_LEN 8
+#define PRODUCT_LEN 16
/* Parse the XML definition for a disk
* @param node XML nodeset to parse for disk definition
@@ -3550,6 +3554,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
char *logical_block_size = NULL;
char *physical_block_size = NULL;
char *wwn = NULL;
+char *vendor = NULL;
+char *product = NULL;
if (VIR_ALLOC(def) 0) {
virReportOOMError();
@@ -3888,6 +3894,24 @@ virDomainDiskDefParseXML(virCapsPtr caps,
if (!virValidateWWN(wwn))
goto error;
+} else if (!vendor
+ xmlStrEqual(cur-name, BAD_CAST vendor)) {
+vendor = (char *)xmlNodeGetContent(cur);
+
+if (strlen(vendor) VENDOR_LEN) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(disk vendor is more than 8 bytes
string));
This should be disk vendor name is more than 8 characters long or ..
is longer than 8 characters, also there is no check these are
alphanumeric although it is mentioned in the documentation. I believe
we can use something similar to virValidateWWN().
+goto error;
+}
+} else if (!product
+ xmlStrEqual(cur-name, BAD_CAST product)) {
+product = (char *)xmlNodeGetContent(cur);
+
+if (strlen(vendor) PRODUCT_LEN) {
+
Re: [libvirt] [PATCH v2] gitignore: ignore more files
On 11/05/2012 05:59 AM, liguang wrote: ignore cscope.in.out, cscope.po.out Signed-off-by: liguang lig.f...@cn.fujitsu.com --- .gitignore |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index 79a055b..12fbe0e 100644 --- a/.gitignore +++ b/.gitignore @@ -188,7 +188,9 @@ Makefile.in TAGS coverage cscope.files +cscope.in.out cscope.out +cscope.po.out results.log stamp-h stamp-h.in ACK Pushed, let us know if you prefer an alternate spelling of your name or preferred email. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Yet another connection fix for 5.1
After the connection to ESX 5.1 being broken since g1e7cd39, the fix in bab7752c helped a bit, but still missed a spot, so the connection is now successful, but some APIs (for example defineXML) don't work. Two cases missing are added in this patch to avoid that. --- src/esx/esx_vi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 097ed48..f3224f8 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2,7 +2,7 @@ /* * esx_vi.c: client for the VMware VI API 2.5 to manage ESX hosts * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2009-2012 Matthias Bolte matthias.bo...@googlemail.com * * This library is free software; you can redistribute it and/or @@ -4657,7 +4657,9 @@ esxVI_ProductVersionToDefaultVirtualHWVersion(esxVI_ProductVersion productVersio case esxVI_ProductVersion_VPX50: return 8; + case esxVI_ProductVersion_ESX51: case esxVI_ProductVersion_ESX5x: + case esxVI_ProductVersion_VPX51: case esxVI_ProductVersion_VPX5x: return 8; -- 1.8.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Yet another connection fix for 5.1
On 11/06/2012 11:00 AM, Peter Krempa wrote: On 11/06/12 10:22, Martin Kletzander wrote: After the connection to ESX 5.1 being broken since g1e7cd39, the fix in bab7752c helped a bit, but still missed a spot, so the connection is now successful, but some APIs (for example defineXML) don't work. Two cases missing are added in this patch to avoid that. --- src/esx/esx_vi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) ACK. Thanks guys, it's pushed now. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] Document bracket whitespace rules add syntax-check rule
On 11/01/2012 11:53 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
[...]
---
build-aux/bracket-spacing.pl | 116
+++
cfg.mk | 7 ++-
docs/hacking.html.in | 49 ++
3 files changed, 171 insertions(+), 1 deletion(-)
create mode 100755 build-aux/bracket-spacing.pl
Even though it is pushed already, as nobody else had a look, I'm going
for it just to have a clean mind.
diff --git a/build-aux/bracket-spacing.pl b/build-aux/bracket-spacing.pl
new file mode 100755
index 000..d3a916f
--- /dev/null
+++ b/build-aux/bracket-spacing.pl
@@ -0,0 +1,116 @@
+#!/usr/bin/perl
+#
+# bracket-spacing.pl: Report any usage of 'function (..args..)'
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library. If not, see
+# http://www.gnu.org/licenses/.
+#
+# Authors:
+# Daniel P. Berrange berra...@redhat.com
+
+use strict;
+use warnings;
+
+my $ret = 0;
+my $incomment = 0;
+
+foreach my $file (@ARGV) {
+open FILE, $file;
+
+while (defined (my $line = FILE)) {
+my $data = $line;
+
+# Kill any quoted strongs
+$data =~ s,.*?,XXX,g;
This doesn't match everything, for example:
printf(_(This \ %s error), func (is));
However, as this is so unlikely to appear, it should be ok as-is. The
only places we have in the code don't usually have a function after
themselves. Checked with:
git grep -E '(^|[^\\])[^\\]*()*\\[^\\]*.*[[:alnum:]_]*\s*\('
[...]
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index d41b39c..37ed00b 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -212,6 +212,55 @@
/p
+h2a name=bracket_spacingBracket spacing/a/h2
+
+p
+ The keywords codeif/code, codefor/code, codewhile/code,
+ and codeswitch/code must have a single space following them
+ before the opening bracket. eg
This would look better with just Example:, IMHO.
[...]
Other than that, everything's ok, I'll add the aesthetic change to my
series of super small cleanups, so post-ACK from me, thanks for the cleanup.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Allow the user to specify vendor and product for disk
On 11/08/2012 02:52 AM, Dave Allan wrote: On Thu, Nov 08, 2012 at 09:28:06AM +0800, Osier Yang wrote: On 2012年11月08日 05:04, Dave Allan wrote: On Wed, Nov 07, 2012 at 11:24:43PM +0800, Osier Yang wrote: On 2012年11月05日 21:34, Martin Kletzander wrote: On 11/05/2012 08:04 AM, Osier Yang wrote: QEMU supports to set vendor and product strings for disk since 1.2.0 (only scsi-disk, scsi-hd, scsi-cd support it), this patch exposes it with new XML elementsvendor andproduct of disk device. --- docs/formatdomain.html.in | 10 + docs/schemas/domaincommon.rng | 10 + src/conf/domain_conf.c | 30 src/conf/domain_conf.h |2 + src/qemu/qemu_command.c| 29 .../qemuxml2argv-disk-scsi-disk-vpd.args | 13 +++ .../qemuxml2argv-disk-scsi-disk-vpd.xml| 36 tests/qemuxml2argvtest.c |4 ++ 8 files changed, 134 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c8da33d..cc9e871 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1657,6 +1657,16 @@ of 16 hexadecimal digits. span class='since'Since 0.10.1/span /dd +dtcodevendor/code/dt +ddIf present, this element specifies the vendor of a virtual hard +disk or CD-ROM device. It's a not more than 8 bytes alphanumeric string. Last sentence doesn't make sense, I suggest changing it to either: It can't be longer than 8 alphanumeric characters. or simply Maximum 8 alphanumeric characters. Okay, honestly, I wasn't comfortable with the sentence, but can't get a better one at that time, :-) I will change your advise a bit: It must not be longer than 8 alphanumeric characters. Not to be pedantic, but what happens if you hand it doublebyte characters? Ah, good question, though QEMU will truncate the string to 8 bytes anyway, should we do some translation in libvirt? the problem is how to map the doublebytes vendors/product to single bytes ones, is there some public specification available? /me starts to think if it's snake leg (or breaking fly on the wheel) to do the checking if it's too heavy. Mostly I was curious about how the code handled doublebyte characters, but now that I think about it, the SCSI spec mandates 8 bytes of ASCII[1], but it sounds like qemu is already enforcing that, so I think it's fine just to let it be freeform and note the spec's requirement in the documentation. I'd say if QEMU starts (and truncates or whatever) with invalid vendor name, there's no problem for us to leave the responsibility on the user, but OTOH when QEMU won't like what it's doing and will eventually fix/change that, our machines may not start. So if we know the limitation (and it is very easy one), why don't we just check (and mention it in the docs) that we accept maximum 8 (16) alphanumeric characters, checking just [A-Za-z0-9] instead of doing some 'isalnum()' magic. Check is easy, QEMU will be always satisfied with the option, I see it as a win-win. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt-1.0 fails virdrivermoduletest
On 11/11/2012 10:25 AM, Toralf Förster wrote: I've a stable Gentoo Linux, the build log is attached. It's hard to say from the log, but you can check yourselves what the problem is by running the particular test like this: VIR_TEST_DEBUG=1 ./virdrivermoduletest in the tests/ directory after compiling the source. That should give you an idea where the problem might be. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Report sensible error for invalid disk name
The error ... but the cause is unknown appeared for XMLs similar to
this:
disk type='file' device='cdrom'
driver name='qemu' type='raw'/
source file='/dev/zero'/
target dev='sr0'/
/disk
Notice unsupported disk type (for the driver), but also no address
specified. The first part is not a problem and we should not abort
immediately because of that, but the combination with the address
unknown was causing an unspecified error.
---
src/util/util.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/util/util.c b/src/util/util.c
index 75b18c1..d5b2c97 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) {
}
}
-if (!ptr)
+if (!ptr) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Unknown disk name '%s' and no address specified),
+ name);
return -1;
+}
for (i = 0; *ptr; i++) {
idx = (idx + (i 1 ? 0 : 1)) * 26;
--
1.8.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Report sensible error for invalid disk name
On 11/20/2012 03:27 PM, Peter Krempa wrote:
On 11/20/12 14:55, Martin Kletzander wrote:
The error ... but the cause is unknown appeared for XMLs similar to
this:
disk type='file' device='cdrom'
driver name='qemu' type='raw'/
source file='/dev/zero'/
target dev='sr0'/
/disk
Notice unsupported disk type (for the driver), but also no address
specified. The first part is not a problem and we should not abort
immediately because of that, but the combination with the address
unknown was causing an unspecified error.
---
src/util/util.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/util/util.c b/src/util/util.c
index 75b18c1..d5b2c97 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) {
}
}
-if (!ptr)
+if (!ptr) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Unknown disk name '%s' and no address
specified),
+ name);
return -1;
+}
Some of the call paths of this function the callers do their own error
reporting (see src/qemu/qemu_command.c) based on the return value. This
should be consolidated.
In case of this function I'd probably rather go for adding the error
message on the appropriate places as the inability to parse the disk ID
is ignored on some calls (maybe that should be fixed in the first place
and than the error reporting is okay here.)
You're right, I went through the code just looking for the un-errored
negative return and haven't thought about that deeply enough. I figured
when the error gets set on two places, it'll be simply shadowed.
Sending a v2 in a minute.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Report sensible error for invalid disk name
On 11/20/2012 03:45 PM, Daniel P. Berrange wrote:
On Tue, Nov 20, 2012 at 02:55:28PM +0100, Martin Kletzander wrote:
The error ... but the cause is unknown appeared for XMLs similar to
this:
disk type='file' device='cdrom'
driver name='qemu' type='raw'/
source file='/dev/zero'/
target dev='sr0'/
/disk
Notice unsupported disk type (for the driver), but also no address
specified. The first part is not a problem and we should not abort
immediately because of that, but the combination with the address
unknown was causing an unspecified error.
---
src/util/util.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/util/util.c b/src/util/util.c
index 75b18c1..d5b2c97 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) {
}
}
-if (!ptr)
+if (!ptr) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Unknown disk name '%s' and no address specified),
+ name);
return -1;
+}
for (i = 0; *ptr; i++) {
idx = (idx + (i 1 ? 0 : 1)) * 26;
IMHO, you should really be adding an ATTRIBUTE_NONNULL annotation and
then fixing the callers not to invoke it if the disk name they have
is Null.
The pointer is not the function attribute, but ...
Adding virReportError here is wrong, because a number of callers will
already report errors.
... you're right as Peter, v2 is on it's way.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] conf: Report sensible error for invalid disk name
The error ... but the cause is unknown appeared for XMLs similar to
this:
disk type='file' device='cdrom'
driver name='qemu' type='raw'/
source file='/dev/zero'/
target dev='sr0'/
/disk
Notice unsupported disk type (for the driver), but also no address
specified. The first part is not a problem and we should not abort
immediately because of that, but the combination with the address
unknown was causing an unspecified error.
While fixing this, I added an error to one place where this return
value was not managed properly.
---
v2:
- Error moved from virDiskNameToIndex@util/util.c to
virDomainDiskDefAssignAddress@conf/domain_conf.c
- One more error added into qemuParseCommandLine@qemu/qemu_command.c
src/conf/domain_conf.c | 6 +-
src/qemu/qemu_command.c | 6 +-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ed21f0f..3a1be02 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3063,8 +3063,12 @@ int
virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def)
{
int idx = virDiskNameToIndex(def-dst);
-if (idx 0)
+if (idx 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Unknown disk name '%s' and no address specified),
+ def-dst);
return -1;
+}
switch (def-bus) {
case VIR_DOMAIN_DISK_BUS_SCSI:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 22bb209..097de9b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8342,8 +8342,12 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
!disk-dst)
goto no_memory;
-if (virDomainDiskDefAssignAddress(caps, disk) 0)
+if (virDomainDiskDefAssignAddress(caps, disk) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Cannot assign address for device name '%s'),
+ disk-dst);
goto error;
+}
if (VIR_REALLOC_N(def-disks, def-ndisks+1) 0)
goto no_memory;
--
1.8.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Correct include-password option for domdisplay
The 'virsh domdisplay' command is able to display the password
configured for spice, but it was missing for vnc type graphics.
This is just a simple patch for that to work properly.
---
tools/virsh-domain.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cc47383..18aa869 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7073,6 +7073,23 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
if (STREQ(scheme[iter], vnc)) {
/* VNC protocol handlers take their port number as 'port' - 5900 */
port -= 5900;
+
+if (vshCommandOptBool(cmd, include-password)) {
+/* Create our XPATH lookup for the SPICE password */
+virAsprintf(xpath,
+string(/domain/devices/graphics
+[@type='%s']/@passwd),
+scheme[iter]);
+if (!xpath) {
+virReportOOMError();
+goto cleanup;
+}
+
+/* Attempt to get the SPICE password */
+passwd = virXPathString(xpath, ctxt);
+VIR_FREE(xpath);
+}
+
} else if (STREQ(scheme[iter], spice)) {
/* Create our XPATH lookup for the SPICE TLS Port */
virAsprintf(xpath, string(/domain/devices/graphics[@type='%s']
--
1.8.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/21/2012 04:39 PM, Daniel P. Berrange wrote:
On Wed, Nov 21, 2012 at 04:37:35PM +0100, Martin Kletzander wrote:
The 'virsh domdisplay' command is able to display the password
configured for spice, but it was missing for vnc type graphics.
This is just a simple patch for that to work properly.
---
tools/virsh-domain.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cc47383..18aa869 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7073,6 +7073,23 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
if (STREQ(scheme[iter], vnc)) {
/* VNC protocol handlers take their port number as 'port' -
5900 */
port -= 5900;
+
+if (vshCommandOptBool(cmd, include-password)) {
+/* Create our XPATH lookup for the SPICE password */
s/spice/vnc/
+virAsprintf(xpath,
+string(/domain/devices/graphics
+[@type='%s']/@passwd),
+scheme[iter]);
+if (!xpath) {
+virReportOOMError();
+goto cleanup;
+}
+
+/* Attempt to get the SPICE password */
s/spice/vnc/
+passwd = virXPathString(xpath, ctxt);
+VIR_FREE(xpath);
+}
+
Daniel
Oh, right, dumb me. I tested the functionality, but left the copy-paste
errors in the comments. Does that mean I can push with those fixes?
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/21/2012 05:06 PM, Ján Tomko wrote: On 11/21/12 16:37, Martin Kletzander wrote: The 'virsh domdisplay' command is able to display the password configured for spice, but it was missing for vnc type graphics. This is just a simple patch for that to work properly. --- tools/virsh-domain.c | 17 + 1 file changed, 17 insertions(+) Wouldn't it be better to put the password before the host (vnc://:passwd@host:port) so that at least some clients (krdc from the few that I've tried) can understand the whole URI? Jan TBH, I don't get this URI-styled remote display definitions because I don't know what program can use those (universally, not just from libvirt, I mean, and I haven't heard about krdc until now) and if there is a recommended scheme for that, however this is how the parameter is appended for spice connections and unless there is a scheme for that and we can say The previous version was a bug, this is how it should be, I don't think we want rick breaking something. OTOH this is just a virsh call, not an API. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/21/2012 11:50 PM, Doug Goldstein wrote: On Wed, Nov 21, 2012 at 10:13 AM, Martin Kletzander mklet...@redhat.com wrote: On 11/21/2012 05:06 PM, Ján Tomko wrote: On 11/21/12 16:37, Martin Kletzander wrote: The 'virsh domdisplay' command is able to display the password configured for spice, but it was missing for vnc type graphics. This is just a simple patch for that to work properly. --- tools/virsh-domain.c | 17 + 1 file changed, 17 insertions(+) Wouldn't it be better to put the password before the host (vnc://:passwd@host:port) so that at least some clients (krdc from the few that I've tried) can understand the whole URI? Jan TBH, I don't get this URI-styled remote display definitions because I don't know what program can use those (universally, not just from libvirt, I mean, and I haven't heard about krdc until now) and if there is a recommended scheme for that, however this is how the parameter is appended for spice connections and unless there is a scheme for that and we can say The previous version was a bug, this is how it should be, I don't think we want rick breaking something. OTOH this is just a virsh call, not an API. Martin I originally did that because we only had vncdisplay with no way to query SPICE info via virsh. So to generically support all graphics protocols I added domdisplay which provides hostname:port like vncdisplay so I needed a way to tell you of the protocol. I figured generic URI RFC style for VNC would be ok since we follow SPICE's URI spec and RDP's URI spec so why special case VNC to not make it a URI. I'll rework it to make it as URI as possible then. Just one question, though (for anyone, I guess); why do we append port as a parameter for spice scheme? Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Report error when taking a snapshot with empty --memspec argument
On 11/22/2012 11:03 AM, Peter Krempa wrote:
When the value of memspec was empty taking of a snapshot failed without
reporting an error.
---
This is only a quick fix. I think we should improve vshCommandOptStr to detect
this for us and report an appropriate error, but this change will require
a lot of changes not relevant to this problem.
I don't think we need to improve that. The function already behaves
depending on the VSH_OFLAG_* being set and returns the right value based
on that. If you meant setting an error, that could be achieved, but
it's already handled in down the stack and even though rewriting it
could save up to 5 lines of code ( :) ), it's better to leave it this
way IMHO.
---
tools/virsh-snapshot.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 398730c..057ae2d 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -358,7 +358,12 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
if (desc)
virBufferEscapeString(buf, description%s/description\n,
desc);
-if (vshCommandOptString(cmd, memspec, memspec) 0 ||
+if (vshCommandOptString(cmd, memspec, memspec) 0) {
+vshError(ctl, _(memspec argument must not be empty));
+goto cleanup;
+}
+
+if (memspec
vshParseSnapshotMemspec(ctl, buf, memspec) 0) {
virBufferFreeAndReset(buf);
goto cleanup;
However, ACK to this change,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Correct include-password option and rewrite domdisplay
On 11/22/2012 02:10 PM, Peter Krempa wrote:
On 11/22/12 11:34, Martin Kletzander wrote:
The 'virsh domdisplay' command is able to display the password
configured for spice, but it was missing for vnc type graphics.
Also, there were some inconsistencies that are cleaned now.
---
[...]
+}
+
/* Then host name or IP */
if (!listen_addr || STREQ((const char *)listen_addr,
0.0.0.0))
virBufferAddLit(buf, localhost);
@@ -7115,20 +7134,11 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
VIR_FREE(listen_addr);
/* Add the port */
-if (STREQ(scheme[iter], spice))
-virBufferAsprintf(buf, ?port=%d, port);
-else
-virBufferAsprintf(buf, :%d, port);
+virBufferAsprintf(buf, :%d, port);
/* TLS Port */
if (tls_port)
-virBufferAsprintf(buf, tls-port=%d, tls_port);
-
-/* Password */
-if (passwd) {
-virBufferAsprintf(buf, password=%s, passwd);
-VIR_FREE(passwd);
-}
+virBufferAsprintf(buf, ?tls-port=%d, tls_port);
/* Ensure we can print our URI */
if (virBufferError(buf)) {
I'm not sure about the change of the password parameter. Could you back
that up somehow?
Unfortunately I cannot. spicy is unable to parse the new version
correctly, but I believe that's a bug since there is a common knowledge
where to put the password. I cooked up a win/win version with the spice
password being printed out the old way and vnc the new (standard) way,
so hopefully everyone could be satisfied, but that seems *very*
inconsistent to me. But since I also shrunk the code a bit more and
fixed one more thing there, I'll send it and let's hope we'll come to
some conclusion then.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] conf: Report sensible error for invalid disk name
On 11/22/2012 02:16 PM, Peter Krempa wrote: On 11/20/12 16:20, Martin Kletzander wrote: The error ... but the cause is unknown appeared for XMLs similar to this: disk type='file' device='cdrom' driver name='qemu' type='raw'/ source file='/dev/zero'/ target dev='sr0'/ /disk Notice unsupported disk type (for the driver), but also no address specified. The first part is not a problem and we should not abort immediately because of that, but the combination with the address unknown was causing an unspecified error. While fixing this, I added an error to one place where this return value was not managed properly. --- v2: - Error moved from virDiskNameToIndex@util/util.c to virDomainDiskDefAssignAddress@conf/domain_conf.c - One more error added into qemuParseCommandLine@qemu/qemu_command.c src/conf/domain_conf.c | 6 +- src/qemu/qemu_command.c | 6 +- 2 files changed, 10 insertions(+), 2 deletions(-) ACK. Peter Thanks, pushed. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cpu: Add Intel Haswell cpu model
On 11/22/2012 03:05 PM, Peter Krempa wrote: The new model supports following features in addition to those supported by SandyBridge: fma, movbe, fsgsbase, bmi1, hle, avx2, smep, bmi2, erms, invpcid, rtm missing 'pcid' flag --- Based on: https://lists.gnu.org/archive/html/qemu-devel/2012-11/msg01328.html --- src/cpu/cpu_map.xml | 16 1 file changed, 16 insertions(+) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 7ff91be..eb69a34 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -495,6 +495,22 @@ feature name='rdtscp'/ /model +model name='Haswell' + model name='SandyBridge'/ + feature name='fma'/ + feature name='pcid'/ + feature name='movbe'/ + feature name='fsgsbase'/ + feature name='bmi1'/ + feature name='hle'/ + feature name='avx2'/ + feature name='smep'/ + feature name='bmi2'/ + feature name='erms'/ + feature name='invpcid'/ + feature name='rtm'/ +/model + !-- AMD CPUs -- model name='athlon' model name='pentiumpro'/ According to the qemu patch, the model should be only adding features, but I see rdtscp disappeared between SandyBridge and Haswell. The question is whether this is QEMU bug or not, do you have any info on that? If not, maybe we should cross-post ask in qemu-devel. We also include 'sep' and 'fpu' on top of these things, but I recall some conversation about qemu dropping 'sep' from some models lately, but I have no idea about 'fpu' flag handling there either. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] virsh: Rewrite cmdDomDisplay
Just a little rewrite of the cmdDomDisplay function to make it
consistent and hopefully more readable. This also fixes a problem
with password not being displayed for vnc even with the
--include-password option.
---
tools/virsh-domain.c | 132 +--
1 file changed, 64 insertions(+), 68 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cc47383..1e8ccc9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7003,9 +7003,9 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool ret = false;
-char *doc;
-char *xpath;
-char *listen_addr;
+char *doc = NULL;
+char *xpath = NULL;
+char *listen_addr = NULL;
int port, tls_port = 0;
char *passwd = NULL;
char *output = NULL;
@@ -7013,6 +7013,8 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
int iter = 0;
int tmp;
int flags = 0;
+bool params = false;
+const char *xpath_fmt = string(/domain/devices/graphics[@type='%s']/@%s);
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -7025,109 +7027,95 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, include-password))
flags |= VIR_DOMAIN_XML_SECURE;
-doc = virDomainGetXMLDesc(dom, flags);
-
-if (!doc)
+if (!(doc = virDomainGetXMLDesc(dom, flags)))
goto cleanup;
-xml = virXMLParseStringCtxt(doc, _((domain_definition)), ctxt);
-VIR_FREE(doc);
-if (!xml)
+if (!(xml = virXMLParseStringCtxt(doc, _((domain_definition)), ctxt)))
goto cleanup;
/* Attempt to grab our display info */
for (iter = 0; scheme[iter] != NULL; iter++) {
/* Create our XPATH lookup for the current display's port */
-virAsprintf(xpath, string(/domain/devices/graphics[@type='%s']
-/@port), scheme[iter]);
-if (!xpath) {
-virReportOOMError();
-goto cleanup;
-}
+if (virAsprintf(xpath, xpath_fmt, scheme[iter], port) 0)
+goto no_memory;
/* Attempt to get the port number for the current graphics scheme */
tmp = virXPathInt(xpath, ctxt, port);
VIR_FREE(xpath);
/* If there is no port number for this type, then jump to the next
- * scheme
- */
+ * scheme */
if (tmp)
continue;
/* Create our XPATH lookup for the current display's address */
-virAsprintf(xpath, string(/domain/devices/graphics[@type='%s']
-/@listen), scheme[iter]);
-if (!xpath) {
-virReportOOMError();
-goto cleanup;
-}
+if (virAsprintf(xpath, xpath_fmt, scheme[iter], listen) 0)
+goto no_memory;
/* Attempt to get the listening addr if set for the current
- * graphics scheme
- */
+ * graphics scheme */
listen_addr = virXPathString(xpath, ctxt);
VIR_FREE(xpath);
-/* Per scheme data mangling */
-if (STREQ(scheme[iter], vnc)) {
-/* VNC protocol handlers take their port number as 'port' - 5900 */
+/* We can query this info for all the graphics types since we'll
+ * get nothing for the unsupported ones (just rdp for now).
+ * Also the parameter '--include-password' was already taken
+ * care of when getting the XML */
+
+/* Create our XPATH lookup for the password */
+if (virAsprintf(xpath, xpath_fmt, scheme[iter], passwd) 0)
+goto no_memory;
+
+/* Attempt to get the password */
+passwd = virXPathString(xpath, ctxt);
+
+if (STREQ(scheme[iter], vnc))
+/* VNC protocol handlers take their port number as
+ * 'port' - 5900 */
port -= 5900;
-} else if (STREQ(scheme[iter], spice)) {
-/* Create our XPATH lookup for the SPICE TLS Port */
-virAsprintf(xpath, string(/domain/devices/graphics[@type='%s']
-/@tlsPort), scheme[iter]);
-if (!xpath) {
-virReportOOMError();
-goto cleanup;
-}
-/* Attempt to get the TLS port number for SPICE */
-tmp = virXPathInt(xpath, ctxt, tls_port);
-VIR_FREE(xpath);
-if (tmp)
-tls_port = 0;
-
-if (vshCommandOptBool(cmd, include-password)) {
-/* Create our XPATH lookup for the SPICE password */
-virAsprintf(xpath, string(/domain/devices/graphics
-[@type='%s']/@passwd), scheme[iter]);
-if (!xpath) {
-virReportOOMError();
-goto cleanup;
-}
+/* Create our XPATH lookup for TLS Port (automatically skipped
+ * for unsupported schemes */
+if
Re: [libvirt] [PATCH 1/6] Fix virDiskNameToIndex to actually ignore partition numbers
On 11/22/2012 05:48 PM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The docs for virDiskNameToIndex claim it ignores partition
numbers. In actual fact though, a code ordering buy means
s/buy/bug/
that a partition number will cause the code to accidentally
multiply the result by 26.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/util.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 75b18c1..2fd0f2c 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2193,11 +2193,10 @@ int virDiskNameToIndex(const char *name) {
return -1;
for (i = 0; *ptr; i++) {
-idx = (idx + (i 1 ? 0 : 1)) * 26;
-
if (!c_islower(*ptr))
break;
+idx = (idx + (i 1 ? 0 : 1)) * 26;
idx += *ptr - 'a';
ptr++;
}
ACK,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cpu: Add Intel Haswell cpu model
On 11/22/2012 04:48 PM, Peter Krempa wrote: On 11/22/12 15:52, Martin Kletzander wrote: On 11/22/2012 03:05 PM, Peter Krempa wrote: [...] According to the qemu patch, the model should be only adding features, but I see rdtscp disappeared between SandyBridge and Haswell. The question is whether this is QEMU bug or not, do you have any info on that? If not, maybe we should cross-post ask in qemu-devel. We also include 'sep' and 'fpu' on top of these things, but I recall some conversation about qemu dropping 'sep' from some models lately, but I have no idea about 'fpu' flag handling there either. Thanks for pointing that out on the qemu-devel list: https://lists.gnu.org/archive/html/qemu-devel/2012-11/msg02400.html I think we can safely assume the patch will make it in a few days, but to be sure I'm giving you an ACK for when the patch gets into qemu's upstream. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] Fix exiting of libvirt_lxc program on container quit
On 11/22/2012 05:48 PM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The virLXCControllerClientCloseHook method was mistakenly
assuming that the private data associated with the network
client was the virLXCControllerPtr. In fact it was just a
dummy int, so we were derefencing a bogus struct. The
frequent result of this was that we would never quit, because
we tried to arm a non-existant timer.
Fix the code by removing the dummy private data and just
using the virLXCControllerPtr instance as private data
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_controller.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 6fffd68..a9d2d40 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -578,19 +578,14 @@ static void
virLXCControllerClientCloseHook(virNetServerClientPtr client)
static void virLXCControllerClientPrivateFree(void *data)
{
-VIR_FREE(data);
+virLXCControllerPtr ctrl = data;
+VIR_DEBUG(Got private data free %p, ctrl);
}
static void *virLXCControllerClientPrivateNew(virNetServerClientPtr client,
void *opaque)
{
virLXCControllerPtr ctrl = opaque;
-int *dummy;
-
-if (VIR_ALLOC(dummy) 0) {
-virReportOOMError();
-return NULL;
-}
virNetServerClientSetCloseHook(client, virLXCControllerClientCloseHook);
VIR_DEBUG(Got new client %p, client);
@@ -600,7 +595,7 @@ static void
*virLXCControllerClientPrivateNew(virNetServerClientPtr client,
virLXCControllerEventSendInit(ctrl, ctrl-initpid);
ctrl-firstClient = false;
-return dummy;
+return ctrl;
}
@@ -1327,7 +1322,7 @@ virLXCControllerEventSendExit(virLXCControllerPtr ctrl,
{
virLXCProtocolExitEventMsg msg;
-VIR_DEBUG(Exit status %d, exitstatus);
+VIR_DEBUG(Exit status %d (client=%p), exitstatus, ctrl-client);
memset(msg, 0, sizeof(msg));
switch (exitstatus) {
case 0:
ACK,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Use virReportSystemError for system error in pci.c
On 11/23/2012 09:02 AM, Osier Yang wrote:
---
src/util/pci.c | 14 ++
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index d1ad121..191f99d 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -1860,10 +1860,9 @@ pciGetPciConfigAddressFromSysfsDeviceLink(const char
*device_link,
device_path = canonicalize_file_name(device_link);
if (device_path == NULL) {
memset(errbuf, '\0', sizeof(errbuf));
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Failed to resolve device link '%s': '%s'),
- device_link, virStrerror(errno, errbuf,
- sizeof(errbuf)));
+virReportSystemError(errno,
+ _(Failed to resolve device link '%s'),
+ device_link);
return ret;
}
@@ -1941,10 +1940,9 @@ pciGetVirtualFunctions(const char *sysfs_path,
dir = opendir(sysfs_path);
if (dir == NULL) {
memset(errbuf, '\0', sizeof(errbuf));
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Failed to open dir '%s': '%s'),
- sysfs_path, virStrerror(errno, errbuf,
- sizeof(errbuf)));
+virReportSystemError(errno,
+ _(Failed to open dir '%s'),
+ sysfs_path);
return ret;
}
These are system errors indeed. ACK,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] Skip deleted timers when calculting next timeout
On 11/22/2012 05:48 PM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
It is possible for there to be deleted timers when we
calculate the next timeout, and they must be skipped.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/event_poll.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/util/event_poll.c b/src/util/event_poll.c
index 2ffa94b..53b9c6c 100644
--- a/src/util/event_poll.c
+++ b/src/util/event_poll.c
@@ -332,6 +332,8 @@ static int virEventPollCalculateTimeout(int *timeout) {
EVENT_DEBUG(Calculate expiry of %zu timers, eventLoop.timeoutsCount);
/* Figure out if we need a timeout */
for (i = 0 ; i eventLoop.timeoutsCount ; i++) {
+if (eventLoop.timeouts[i].deleted)
+continue;
if (eventLoop.timeouts[i].frequency 0)
continue;
ACK,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/23/2012 09:20 AM, Christophe Fergeau wrote: On Thu, Nov 22, 2012 at 11:32:38AM +0100, Ján Tomko wrote: On 11/22/12 11:03, Martin Kletzander wrote: I'll rework it to make it as URI as possible then. Just one question, though (for anyone, I guess); why do we append port as a parameter for spice scheme? Martin I think it's because of what spicy supported initially (since v0.1.0): http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=f0693b9f949ba spice://host:port is supported since v0.8 http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=b4c72ece9ca3b and spice://host:port/ (with the trailing slash) since v0.12 http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=50add15ef69cd Another thing to keep in mind is that you can have both a port and a secure port (over which data will transit through SSL). Christophe I'm keeping that in the parameter as there will be both port and tlsPort available in this case. I also modified it so we have a vnc version and spice version of the output, which can be seen in v2 (rewrite cmdDomDisplay). Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] virsh: Report error when taking a snapshot with empty --memspec argument
On 11/22/2012 02:41 PM, Peter Krempa wrote:
When the value of memspec was empty taking of a snapshot failed without
reporting an error.
---
tools/virsh-snapshot.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 398730c..8ec6456 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -358,18 +358,19 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
if (desc)
virBufferEscapeString(buf, description%s/description\n,
desc);
-if (vshCommandOptString(cmd, memspec, memspec) 0 ||
-vshParseSnapshotMemspec(ctl, buf, memspec) 0) {
-virBufferFreeAndReset(buf);
+if (vshCommandOptString(cmd, memspec, memspec) 0) {
+vshError(ctl, _(memspec argument must not be empty));
goto cleanup;
}
+
+if (memspec vshParseSnapshotMemspec(ctl, buf, memspec) 0)
+goto cleanup;
+
if (vshCommandOptBool(cmd, diskspec)) {
virBufferAddLit(buf, disks\n);
while ((opt = vshCommandOptArgv(cmd, opt))) {
-if (vshParseSnapshotDiskspec(ctl, buf, opt-data) 0) {
-virBufferFreeAndReset(buf);
+if (vshParseSnapshotDiskspec(ctl, buf, opt-data) 0)
goto cleanup;
-}
}
virBufferAddLit(buf, /disks\n);
}
@@ -390,6 +391,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
ret = vshSnapshotCreate(ctl, dom, buffer, flags, NULL);
cleanup:
+virBufferFreeAndReset(buf);
VIR_FREE(buffer);
if (dom)
virDomainFree(dom);
I've missed the leak in the first version, thanks for finding that out.
I double-checked this one and it seems alright, so ACK.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: fix crash when portgroup has no name
On 11/28/2012 06:08 AM, Laine Stump wrote:
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=879473
The name attribute is required for portgroup elements (yes, the RNG
specifies that), and there is code in libvirt that assumes it is
non-null. Unfortunately, the portgroup parsing function wasn't
checking for lack of portgroup. One adverse result of this was that
attempts to update a network by adding a portgroup with no name would
cause libvirtd to segfault. For example:
virsh net-update default add portgroup portgroup default='yes'/
This patch causes virNetworkPortGroupParseXML to fail if no name is
specified, thus avoiding any later problems.
Looking at the code, I see it's required on more places, yes. And
according to the documentation the name is needed.
---
src/conf/network_conf.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 228951d..6ce2e63 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1175,6 +1175,12 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def,
/* grab raw data from XML */
def-name = virXPathString(string(./@name), ctxt);
+if (!def-name) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Missing required name attribute in portgroup));
+goto error;
+}
+
isDefault = virXPathString(string(./@default), ctxt);
def-isDefault = isDefault STRCASEEQ(isDefault, yes);
Just a question; there's a similar check for (!def-name), for networks
particularly, and that one uses VIR_ERR_NO_NAME (specified as a error
for missing _domain_ name). Should one of these be changed (in a
separate patch, of course)?
Anyway, ACK for this one,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix a crash when save file can't be opened
On 11/28/2012 09:08 AM, Ján Tomko wrote:
In qemuDomainSaveMemory, wrapperFd might be NULL and should be checked before
calling virFileWrapperFdCatchError. Same in doCoreDump.
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=880919
---
src/qemu/qemu_driver.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c526f5f..7892293 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2906,7 +2906,8 @@ qemuDomainSaveMemory(struct qemud_driver *driver,
cleanup:
VIR_FORCE_CLOSE(fd);
-virFileWrapperFdCatchError(wrapperFd);
+if (wrapperFd)
+virFileWrapperFdCatchError(wrapperFd);
virFileWrapperFdFree(wrapperFd);
VIR_FREE(xml);
@@ -3362,7 +3363,8 @@ doCoreDump(struct qemud_driver *driver,
cleanup:
VIR_FORCE_CLOSE(fd);
if (ret != 0) {
-virFileWrapperFdCatchError(wrapperFd);
+if (wrapperFd)
+virFileWrapperFdCatchError(wrapperFd);
unlink(path);
}
virFileWrapperFdFree(wrapperFd);
ACK Pushed,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: more fix to avoid C99 for loop (Re: [PATCH] build: avoid C99 for loop)
On 11/28/2012 04:37 AM, Hu Tao wrote:
On Mon, Nov 26, 2012 at 03:25:04PM -0700, Eric Blake wrote:
Although we require various C99 features, we don't yet require a
complete C99 compiler. On RHEL 5, compilation complained:
qemu/qemu_command.c: In function 'qemuBuildGraphicsCommandLine':
qemu/qemu_command.c:4688: error: 'for' loop initial declaration used outside
C99 mode
* src/qemu/qemu_command.c (qemuBuildGraphicsCommandLine): Declare
variable sooner.
* src/qemu/qemu_process.c (qemuProcessInitPasswords): Likewise.
find ./ -name '*.c' | xargs grep 'for *( *int '
reveals another file, see the patch below.
From 9f5f9112108c5ab42e56c1e4e9db185d7dfb6cf4 Mon Sep 17 00:00:00 2001
From: Hu Tao hu...@cn.fujitsu.com
Date: Wed, 28 Nov 2012 11:31:26 +0800
Subject: [PATCH] build: more fix to avoid C99 for loop
see commit 7e5aa78d0f7f4cbf1c8
* src/interface/interface_backend_udev.c: Declare variable sooner.
---
src/interface/interface_backend_udev.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index 5a27cc5..ed73d54 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -515,12 +515,14 @@ udevIfaceScanDirFilter(const struct dirent *entry)
static void
udevIfaceFreeIfaceDef(virInterfaceDef *ifacedef)
{
+int i;
+
if (!ifacedef)
return;
if (ifacedef-type == VIR_INTERFACE_TYPE_BRIDGE) {
VIR_FREE(ifacedef-data.bridge.delay);
-for (int i = 0; i ifacedef-data.bridge.nbItf; i++) {
+for (i = 0; i ifacedef-data.bridge.nbItf; i++) {
udevIfaceFreeIfaceDef(ifacedef-data.bridge.itf[i]);
}
VIR_FREE(ifacedef-data.bridge.itf);
@@ -547,6 +549,7 @@ udevIfaceGetIfaceDef(struct udev *udev, char *name)
char *vlan_parent_dev = NULL;
struct dirent **member_list = NULL;
int member_count = 0;
+int i;
/* Allocate our interface definition structure */
if (VIR_ALLOC(ifacedef) 0) {
@@ -679,7 +682,7 @@ udevIfaceGetIfaceDef(struct udev *udev, char *name)
}
ifacedef-data.bridge.nbItf = member_count;
-for (int i= 0; i member_count; i++) {
+for (i= 0; i member_count; i++) {
Ewww, this could use a space: s/i=/i =/
ifacedef-data.bridge.itf[i] =
udevIfaceGetIfaceDef(udev, member_list[i]-d_name);
VIR_FREE(member_list[i]);
@@ -698,7 +701,7 @@ udevIfaceGetIfaceDef(struct udev *udev, char *name)
cleanup:
udev_device_unref(dev);
-for (int i = 0; i member_count; i++) {
+for (i = 0; i member_count; i++) {
VIR_FREE(member_list[i]);
}
VIR_FREE(member_list);
ACK with the space fixed (and the obvious: no '(Re:...' in the commit
message),
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: Rewrite cmdDomDisplay
On 11/28/2012 02:38 PM, Peter Krempa wrote:
On 11/22/12 16:24, Martin Kletzander wrote:
[...]
+/* We can query this info for all the graphics types since we'll
+ * get nothing for the unsupported ones (just rdp for now).
+ * Also the parameter '--include-password' was already taken
+ * care of when getting the XML */
+
+/* Create our XPATH lookup for the password */
+if (virAsprintf(xpath, xpath_fmt, scheme[iter], passwd) 0)
+goto no_memory;
+
+/* Attempt to get the password */
+passwd = virXPathString(xpath, ctxt);
You forgot to VIR_FREE(xpath) here leaking one of the query strings.
+
+if (STREQ(scheme[iter], vnc))
+/* VNC protocol handlers take their port number as
+ * 'port' - 5900 */
port -= 5900;
-} else if (STREQ(scheme[iter], spice)) {
-/* Create our XPATH lookup for the SPICE TLS Port */
-virAsprintf(xpath,
[...]
Nice cleanup. ACK with the memleak fixed.
Peter
I fixed the leak and added curly braces around multi-line block after
that. Long story short, I pushed it with the following squashed.
Martin
---
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1e8ccc9..1723413 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7068,11 +7068,13 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
/* Attempt to get the password */
passwd = virXPathString(xpath, ctxt);
+VIR_FREE(xpath);
-if (STREQ(scheme[iter], vnc))
+if (STREQ(scheme[iter], vnc)) {
/* VNC protocol handlers take their port number as
* 'port' - 5900 */
port -= 5900;
+}
/* Create our XPATH lookup for TLS Port (automatically skipped
* for unsupported schemes */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: fix crash when portgroup has no name
On 11/28/2012 05:55 PM, Laine Stump wrote:
On 11/28/2012 04:19 AM, Martin Kletzander wrote:
On 11/28/2012 06:08 AM, Laine Stump wrote:
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=879473
The name attribute is required for portgroup elements (yes, the RNG
specifies that), and there is code in libvirt that assumes it is
non-null. Unfortunately, the portgroup parsing function wasn't
checking for lack of portgroup. One adverse result of this was that
attempts to update a network by adding a portgroup with no name would
cause libvirtd to segfault. For example:
virsh net-update default add portgroup portgroup default='yes'/
This patch causes virNetworkPortGroupParseXML to fail if no name is
specified, thus avoiding any later problems.
Looking at the code, I see it's required on more places, yes. And
according to the documentation the name is needed.
---
src/conf/network_conf.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 228951d..6ce2e63 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1175,6 +1175,12 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def,
/* grab raw data from XML */
def-name = virXPathString(string(./@name), ctxt);
+if (!def-name) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Missing required name attribute in portgroup));
+goto error;
+}
+
isDefault = virXPathString(string(./@default), ctxt);
def-isDefault = isDefault STRCASEEQ(isDefault, yes);
Just a question; there's a similar check for (!def-name), for networks
particularly, and that one uses VIR_ERR_NO_NAME (specified as a error
for missing _domain_ name). Should one of these be changed (in a
separate patch, of course)?
According to the comment with the definition of VIR_ERR_NO_NAME, it's
intended only for when the name is missing from a domain. The error
printed out though is missing name information (with optional in %s).
So if you look at the comment, it seems that it shouldn't be used for
network (or node device) name, but if you look at the message generated,
it looks like it could be used for any missing name. Personally I don't
see the use of having a separate error code for missing name vs. missing
[anything else]. To me it's just like any other XML error.
That's exactly what I meant, so you assured me that I haven't
misunderstood the second appearance of that.
Thanks,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: add VMmanager to web apps
This is a request for adding a VMmanager application as requested and described by Ksenya Phil. Signed-off-by: Ksenya Phil philka2...@mail.ru Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/apps.html.in | 9 + 1 file changed, 9 insertions(+) diff --git a/docs/apps.html.in b/docs/apps.html.in index 7b581db..86e45fc 100644 --- a/docs/apps.html.in +++ b/docs/apps.html.in @@ -393,6 +393,15 @@ with FreeIPA for Kerberos authentication, and in the future, certificate management. /dd + dta href=http://ispsystem.com/en/software/vmmanager;VMmanager/a/dt + dd +VMmanager is a software solution for virtualization management +that can be used both for hosting virtual machines and +building a cloud. VMmanager can manage not only one server, +but a large cluster of hypervisors. It delivers a number of +functions, such as live migration that allows for load +balancing between cluster nodes, monitoring CPU, memory. + /dd /dl h2a name=mobileMobile applications/a/h2 -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 1/3] add new virDomainCoreDumpWithFormat API
On Thu, Mar 06, 2014 at 09:35:47AM +, qiaonuo...@cn.fujitsu.com wrote:
--memory-only option is introduced without compression supported. Therefore,
this is a freature regression of virsh dump. Now qemu has support dumping
memory
s/freature/feature/
but I would not use the word regression since that never worked.
Also it would help mentioning the commit ID or a version it
got included in qemu. On that note, is there a possibility of
of introspection of that feature, so we can gracefully error out in
case older qemu is used?
in kdump-compressed format. This patch is adding new
virDomainCoreDumpWithFormat
API, so that the format in which qemu dump domain's memory can be specified.
s/dump/dumps/
Looking at the rest, I rather fixed what I wanted to change in my repo
and here's the diff I'd squash in. Let me know if you're OK with
that. I'll still want an ACK from someone in order to push that,
though. And feel free to ask about that changes as well.
Martin
diff --git c/include/libvirt/libvirt.h.in i/include/libvirt/libvirt.h.in
index 12d64ab..41cd28c 100644
--- c/include/libvirt/libvirt.h.in
+++ i/include/libvirt/libvirt.h.in
@@ -1186,15 +1186,15 @@ typedef enum {
* Values for specifying different formats of domain core dumps.
*/
typedef enum {
-VIR_DUMP_FORMAT_RAW, /* dump guest memory in raw format */
-VIR_DUMP_FORMAT_KDUMP_ZLIB, /* dump guest memory in kdump-compressed
- format, with zlib-compressed */
-VIR_DUMP_FORMAT_KDUMP_LZO,/* dump guest memory in kdump-compressed
- format, with lzo-compressed */
-VIR_DUMP_FORMAT_KDUMP_SNAPPY, /* dump guest memory in kdump-compressed
- format, with snappy-compressed */
+VIR_DOMAIN_CORE_DUMP_FORMAT_RAW, /* dump guest memory in raw
format */
+VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_ZLIB, /* kdump-compressed format, with
+ * zlib compression */
+VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_LZO,/* kdump-compressed format, with
+ * lzo compression */
+VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_SNAPPY, /* kdump-compressed format, with
+ * snappy compression */
#ifdef VIR_ENUM_SENTINELS
-VIR_DUMP_FORMAT_LAST
+VIR_DOMAIN_CORE_DUMP_FORMAT_LAST
/*
* NB: this enum value will increase over time as new events are
* added to the libvirt API. It reflects the last state supported
@@ -1756,10 +1756,10 @@ int virDomainCoreDump
(virDomainPtr domain,
/*
* Domain core dump with format specified
*/
-int virDomainCoreDumpWithFormat (virDomainPtr domain,
- const char *to,
- unsigned int dumpformat,
- unsigned int flags);
+int virDomainCoreDumpWithFormat (virDomainPtr domain,
+ const char *to,
+ unsigned int dumpformat,
+ unsigned int flags);
/*
* Screenshot of current domain console
diff --git c/src/libvirt.c i/src/libvirt.c
index cb8f0d2..a4787a8 100644
--- c/src/libvirt.c
+++ i/src/libvirt.c
@@ -3032,7 +3032,7 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const
char *to, unsigned int
{
virConnectPtr conn;
-VIR_DOMAIN_DEBUG(domain, to=%s, flags=%x, to, flags);
+VIR_DOMAIN_DEBUG(domain, to=%s, dumpformat=%u, flags=%x, to, dumpformat,
flags);
virResetLastError();
@@ -3042,7 +3042,7 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const
char *to, unsigned int
virCheckReadOnlyGoto(conn-flags, error);
virCheckNonNullArgGoto(to, error);
-if (dumpformat = VIR_DUMP_FORMAT_LAST) {
+if (dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_LAST) {
virReportInvalidArg(flags, _(dumpformat '%d' is not supproted),
dumpformat);
goto error;
@@ -3056,7 +3056,7 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const
char *to, unsigned int
if ((flags VIR_DUMP_CRASH) (flags VIR_DUMP_RESET)) {
virReportInvalidArg(flags, %s,
- _(crash and reset flags are mutually exclusive));
+_(crash and reset flags are mutually exclusive));
goto error;
}
diff --git c/src/remote_protocol-structs i/src/remote_protocol-structs
index 0e2101c..456d0da 100644
--- c/src/remote_protocol-structs
+++ i/src/remote_protocol-structs
@@ -560,7 +560,7 @@ struct remote_domain_core_dump_args {
struct remote_domain_core_dump_with_format_args {
remote_nonnull_domain dom;
remote_nonnull_string to;
-u_int dompformat;
+
Re: [libvirt] [PATCH v5 2/3] add qemu support to virDomainCoreDumpWithFormat API
On Thu, Mar 06, 2014 at 09:35:47AM +, qiaonuo...@cn.fujitsu.com wrote:
s/to/for/ in the $SUBJ or you can rephrase it to:
qemu: add support for virDomainCoreDumpWithFormat API
This patch makes qemu driver supprot virDomainCoreDumpWithFormat API.
s/supprot/support/
And again like with previous patch, here's a diff of what I'd change.
For this one I'm still missing the detection of kdump-compressed dump
capability in qemu.
Martin
diff --git c/src/qemu/qemu_driver.c i/src/qemu/qemu_driver.c
index 995cae0..ee8fcf9 100644
--- c/src/qemu/qemu_driver.c
+++ i/src/qemu/qemu_driver.c
@@ -2676,6 +2676,13 @@ VIR_ENUM_IMPL(qemuSaveCompression, QEMU_SAVE_FORMAT_LAST,
xz,
lzop)
+VIR_ENUM_DECL(qemuDumpFormat)
+VIR_ENUM_IMPL(qemuDumpFormat, VIR_DOMAIN_CORE_DUMP_FORMAT_LAST,
+ elf,
+ kdump-zlib,
+ kdump-lzo,
+ kdump-snappy)
+
typedef struct _virQEMUSaveHeader virQEMUSaveHeader;
typedef virQEMUSaveHeader *virQEMUSaveHeaderPtr;
struct _virQEMUSaveHeader {
@@ -3453,25 +3460,20 @@ doCoreDump(virQEMUDriverPtr driver,
goto cleanup;
if (dump_flags VIR_DUMP_MEMORY_ONLY) {
-if (dumpformat == VIR_DUMP_FORMAT_RAW)
-memory_dump_format = elf;
-else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_ZLIB)
-memory_dump_format = kdump-zlib;
-else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_LZO)
-memory_dump_format = kdump-lzo;
-else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_SNAPPY)
-memory_dump_format = kdump-snappy;
-else {
+if (!(memory_dump_format = qemuDumpFormatTypeToString(dumpformat))) {
virReportError(VIR_ERR_INVALID_ARG,
_(unknown dumpformat '%d'), dumpformat);
+goto cleanup;
}
ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP,
memory_dump_format);
} else {
-if (dumpformat != VIR_DUMP_FORMAT_RAW)
+if (dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_RAW) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
- _(kdump-compressed format is only work with guest
- memory dump));
+ _(kdump-compressed format is only supported with
+ memory-only dump));
+goto cleanup;
+}
ret = qemuMigrationToFile(driver, vm, fd, 0, path,
qemuCompressProgramName(compress), false,
QEMU_ASYNC_JOB_DUMP);
@@ -3643,7 +3645,8 @@ static int qemuDomainCoreDump(virDomainPtr dom,
const char *path,
unsigned int flags)
{
-return qemuDomainCoreDumpWithFormat(dom, path, VIR_DUMP_FORMAT_RAW, flags);
+return qemuDomainCoreDumpWithFormat(dom, path,
+VIR_DOMAIN_CORE_DUMP_FORMAT_RAW,
flags);
}
static char *
@@ -3770,7 +3773,7 @@ static void processWatchdogEvent(virQEMUDriverPtr driver,
virDomainObjPtr vm, in
flags |= cfg-autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0;
ret = doCoreDump(driver, vm, dumpfile,
getCompressionType(driver), flags,
- VIR_DUMP_FORMAT_RAW);
+ VIR_DOMAIN_CORE_DUMP_FORMAT_RAW);
if (ret 0)
virReportError(VIR_ERR_OPERATION_FAILED,
%s, _(Dump failed));
@@ -3834,7 +3837,8 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver,
flags |= cfg-autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0;
ret = doCoreDump(driver, vm, dumpfile,
- getCompressionType(driver), flags, VIR_DUMP_FORMAT_RAW);
+ getCompressionType(driver), flags,
+ VIR_DOMAIN_CORE_DUMP_FORMAT_RAW);
if (ret 0)
virReportError(VIR_ERR_OPERATION_FAILED,
%s, _(Dump failed));
--
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 3/3] allow virsh dump --memory-only specify dump format
On Thu, Mar 06, 2014 at 09:35:47AM +, qiaonuo...@cn.fujitsu.com wrote:
This patch is used to add --compress and [--compression-format] string
to
s/is used to add/adds/
virsh dump --memory-only. And virsh dump --memory-only is going be
implemented by new virDomainCoreDumpWithFormat API.
How about rewording it like this:
... to virsh dump --memory-only, which is changed to use the new
virDomainCoreDumpWithFormat API.
The same about the following diff as with the first patch applies.
Martin
diff --git c/tools/virsh-domain.c w/tools/virsh-domain.c
index f8e8e57..51881ef 100644
--- c/tools/virsh-domain.c
+++ w/tools/virsh-domain.c
@@ -4487,8 +4487,8 @@ static const vshCmdOptDef opts_dump[] = {
.help = N_(dump domain's memory only)
},
{.name = compress,
- .type = VSH_OT_BOOL,
- .help = N_(make qemu dump domain's memory in kdump-compressed format)
+ .type = VSH_OT_ALIAS,
+ .help = compression-format=zlib,
},
{.name = compression-format,
.type = VSH_OT_DATA,
@@ -4509,9 +4509,8 @@ doDump(void *opaque)
const char *name = NULL;
const char *to = NULL;
unsigned int flags = 0;
-bool optCompress;
const char *compression_format = NULL;
-unsigned int memory_dump_format = VIR_DUMP_FORMAT_RAW;
+unsigned int memory_dump_format = VIR_DOMAIN_CORE_DUMP_FORMAT_RAW;
sigemptyset(sigmask);
sigaddset(sigmask, SIGINT);
@@ -4535,36 +4534,26 @@ doDump(void *opaque)
if (vshCommandOptBool(cmd, memory-only))
flags |= VIR_DUMP_MEMORY_ONLY;
-optCompress = vshCommandOptBool(cmd, compress);
-if (optCompress !(flags VIR_DUMP_MEMORY_ONLY)) {
+if (vshCommandOptBool(cmd, compression-format)
+!(flags VIR_DUMP_MEMORY_ONLY)) {
vshError(ctl, %s,
_(compress flag cannot be set without memory-only flag));
goto out;
}
if (vshCommandOptString(cmd, compression-format, compression_format)) {
-if (!optCompress) {
-vshError(ctl, %s,
- _(compression-format cannot be set without compress
- flag));
-goto out;
-}
-
if (STREQ(compression_format, zlib))
-memory_dump_format = VIR_DUMP_FORMAT_KDUMP_ZLIB;
+memory_dump_format = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_ZLIB;
else if (STREQ(compression_format, lzo))
-memory_dump_format = VIR_DUMP_FORMAT_KDUMP_LZO;
+memory_dump_format = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_LZO;
else if (STREQ(compression_format, snappy))
-memory_dump_format = VIR_DUMP_FORMAT_KDUMP_SNAPPY;
+memory_dump_format = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_SNAPPY;
else {
vshError(ctl, _(compression format '%s' is not supported,
expecting 'zlib', 'lzo' or 'snappy'.),
compression_format);
goto out;
}
-} else {
-if (optCompress)
-memory_dump_format = VIR_DUMP_FORMAT_KDUMP_ZLIB;
}
if (virDomainCoreDumpWithFormat(dom, to, memory_dump_format, flags) 0) {
diff --git c/tools/virsh.pod w/tools/virsh.pod
index cefce1b..0eb7ac6 100644
--- c/tools/virsh.pod
+++ w/tools/virsh.pod
@@ -995,6 +995,7 @@ Iformat argument may be Bxen-xm or Bxen-sxpr.
=item Bdump Idomain Icorefilepath [I--bypass-cache]
{ [I--live] | [I--crash] | [I--reset] } [I--verbose] [I--memory-only]
+[I--compression-format Istring]
Dumps the core of a domain to a file for analysis.
If I--live is specified, the domain continues to run until the core
@@ -1008,6 +1009,10 @@ cache, although this may slow down the operation.
If I--memory-only is specified, the file is elf file, and will only
include domain's memory and cpu common register value. It is very
useful if the domain uses host devices directly.
+I--compression-format Istring can be used to specify that the
+output file should be in kdump-compressed format using the specified
+compression; zlib, lzo, snappy.
+I--compress is an alias for I--compression-format Ilzo.
The progress may be monitored using Bdomjobinfo virsh command and canceled
with Bdomjobabort command (sent by another virsh instance). Another option
--
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-python PATCH] setup: Make libvirt API XML path configurable
On Wed, Mar 12, 2014 at 11:15:43AM -0600, Eric Blake wrote: Revisiting an older thread On 11/26/2013 07:38 AM, Martin Kletzander wrote: On Tue, Nov 26, 2013 at 10:14:36AM +, Daniel P. Berrange wrote: On Tue, Nov 26, 2013 at 10:58:25AM +0100, Martin Kletzander wrote: Adding a support for LIBVIRT_API_PATH evironment variable, which can control where the script should look for the 'libvirt-api.xml' file. This allows building libvirt-python against different libvirt than the one installed in the system. This may be used for example in autotest or by packagers without the need to install libvirt into the system. -libvirt_api = get_pkgconfig_data([--variable, libvirt_api], libvirt) +libvirt_api = os.getenv(LIBVIRT_API_PATH) + NACK, setting pkg-config already takes care of this. See the build-many.sh scrpit attached to this mail which demonstrates use of PKG_CONFIG_PATH to build against every version of libvirt back to 0.9.11 This still means you have to configure libvirt with different prefix, install it and then you can use PKG_CONFIG_PATH. This variable (which is unused if unset) makes it easier to use in case you have it built with default prefix etc. It would help me a lot, but if everyone else is OK with installing libvirt in order to build python bindings just to test something, I'll keep this in my git. I'm still interested in the ability to test libvirt-python against an uninstalled libvirt tree. Should we revisit this patch, or something like it? I'd be still happy to have this in, especially for us developers. It's useful when testing a libvirt-python patch against upstream libvirt which, in this case, doesn't have to be installed to make the bindings work. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: add VMmanager to web apps
On Wed, Mar 12, 2014 at 06:57:30AM -0600, Eric Blake wrote: On 03/12/2014 01:30 AM, Martin Kletzander wrote: This is a request for adding a VMmanager application as requested and described by Ksenya Phil. Signed-off-by: Ksenya Phil philka2...@mail.ru Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/apps.html.in | 9 + 1 file changed, 9 insertions(+) ACK Pushed, thanks. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 3/3] allow virsh dump --memory-only specify dump format
On Wed, Mar 12, 2014 at 05:18:22PM +, Daniel P. Berrange wrote:
On Thu, Mar 06, 2014 at 09:35:47AM +, qiaonuo...@cn.fujitsu.com wrote:
This patch is used to add --compress and [--compression-format]
string to
virsh dump --memory-only. And virsh dump --memory-only is going be
implemented by new virDomainCoreDumpWithFormat API.
Signed-off-by: Qiao Nuohan qiaonuo...@cn.fujitsu.com
---
tools/virsh-domain.c | 45 -
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2e3f0ed..70613e5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4486,6 +4486,14 @@ static const vshCmdOptDef opts_dump[] = {
.type = VSH_OT_BOOL,
.help = N_(dump domain's memory only)
},
+{.name = compress,
+ .type = VSH_OT_BOOL,
+ .help = N_(make qemu dump domain's memory in kdump-compressed
format)
+},
+{.name = compression-format,
+ .type = VSH_OT_DATA,
+ .help = N_(specify the compression format of kdump-compressed
format)
+},
As before, IMHO having two args here is silly. Just have a
single '--compress format' arg.
I'm fine with having one param only, I don't know about the author,
though. I also proposed '--compress' as an alias which should be good
compromise IMHO.
@@ -4524,7 +4535,39 @@ doDump(void *opaque)
if (vshCommandOptBool(cmd, memory-only))
flags |= VIR_DUMP_MEMORY_ONLY;
-if (virDomainCoreDump(dom, to, flags) 0) {
[snip]
+if (virDomainCoreDumpWithFormat(dom, to, memory_dump_format, flags)
0) {
This breaks virsh dump if used against an older libvirtd.
We must *only* invoke virDomainCoreDumpWithFormat if the user
has specified a 'memory_dump_format' value
That's right, I forgot that. That should be definitely checked for
and it must properly error out.
Martin
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 1/3] add new virDomainCoreDumpWithFormat API
On Wed, Mar 12, 2014 at 09:29:55AM -0600, Eric Blake wrote: On 03/12/2014 09:04 AM, Martin Kletzander wrote: On Thu, Mar 06, 2014 at 09:35:47AM +, qiaonuo...@cn.fujitsu.com wrote: --memory-only option is introduced without compression supported. Therefore, this is a freature regression of virsh dump. Now qemu has support dumping memory s/freature/feature/ but I would not use the word regression since that never worked. Also it would help mentioning the commit ID or a version it got included in qemu. On that note, is there a possibility of of introspection of that feature, so we can gracefully error out in case older qemu is used? Yes - qemu 2.0 is adding 'query-dump-guest-memory-capability', which can be used for two purposes: 1. if this command exists, 'dump-guest-memory' supports the new optional 'format' argument; and 2. calling this command will return a list of WHICH formats are supported by the given qemu binary (since configure-time choices can disable some of the formats from actually working). So you need to have a patch that modifies src/qemu/qemu_capabilities.[ch] to do the probing and set capability bits, so that we can gracefully error out when talking to a too-old qemu, or requesting a format that was not compiled in to a new qemu. Great. Could we have the compression parameter just as an arbitrary string then (properly checked for, etc.) so there is no need to adapt our code to qemu format additions? Looking at the rest, I rather fixed what I wanted to change in my repo and here's the diff I'd squash in. Let me know if you're OK with that. I'll still want an ACK from someone in order to push that, though. And feel free to ask about that changes as well. I suppose the capability detection could be done as an add-on patch, but I'm personally thinking it's better to hold off on this series until ALL the work is done, so we don't forget to do the capability detection work. Definitely, I just asked this question in the wrong patch, the detection and use of the capability should be in 2/3 or in separate one. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 3/3] allow virsh dump --memory-only specify dump format
On Thu, Mar 13, 2014 at 09:22:30AM +, qiaonuo...@cn.fujitsu.com wrote:
On 03/13/2014 03:40 PM, Martin Kletzander wrote:
On Wed, Mar 12, 2014 at 05:18:22PM +, Daniel P. Berrange wrote:
On Thu, Mar 06, 2014 at 09:35:47AM +,qiaonuo...@cn.fujitsu.com
wrote:
This patch is used to add --compress and
[--compression-format]string to
virsh dump --memory-only. And virsh dump --memory-only is going
be
implemented by new virDomainCoreDumpWithFormat API.
Signed-off-by: Qiao Nuohanqiaonuo...@cn.fujitsu.com
---
tools/virsh-domain.c | 45
-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2e3f0ed..70613e5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4486,6 +4486,14 @@ static const vshCmdOptDef opts_dump[] = {
.type = VSH_OT_BOOL,
.help = N_(dump domain's memory only)
},
+{.name = compress,
+ .type = VSH_OT_BOOL,
+ .help = N_(make qemu dump domain's memory in
kdump-compressed format)
+},
+{.name = compression-format,
+ .type = VSH_OT_DATA,
+ .help = N_(specify the compression format of
kdump-compressed format)
+},
As before, IMHO having two args here is silly. Just have a
single '--compress format' arg.
I'm fine with having one param only, I don't know about the author,
though. I also proposed '--compress' as an alias which should be good
compromise IMHO.
I would prefer Martin's suggestion, for zlib is used more frequently, an alias
can save typing.
Although one might suggest an alias or a wrapper if you want to save
typing...
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 1/3] add new virDomainCoreDumpWithFormat API
On Thu, Mar 13, 2014 at 09:20:09AM +, qiaonuo...@cn.fujitsu.com wrote:
On 03/12/2014 11:04 PM, Martin Kletzander wrote:
diff --git c/src/test/test_driver.c i/src/test/test_driver.c
index 39b3066..20f7bb3 100644
--- c/src/test/test_driver.c
+++ i/src/test/test_driver.c
@@ -2436,6 +2436,13 @@ static int testDomainCoreDumpWithFormat(virDomainPtr
domain,
virCheckFlags(VIR_DUMP_CRASH, -1);
+/* we don't support non-raw formats in test driver */
+if (dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_RAW) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(kdump-compressed format is not supported here));
+goto cleanup;
+}
+
Moving the check for dumpformat here may jump to cleanup before privdom is
initialize. So is it suitable moving this check before the check of flags?
like
I just wanted to check and error out properly before doing the work
and not after that, so that's ok, yes.
Martin
cut
@@ -2467,6 +2468,13 @@ static int testDomainCoreDump(virDomainPtr domain,
goto cleanup;
}
+/* we don't support non-raw formats in test driver */
+if (dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_RAW) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(kdump-compressed format is not supported here));
+goto cleanup;
+}
+
if (flags VIR_DUMP_CRASH) {
testDomainShutdownState(domain, privdom,
VIR_DOMAIN_SHUTOFF_CRASHED);
event = virDomainEventLifecycleNewFromObj(privdom,
cut
testDriverLock(privconn);
privdom = virDomainObjListFindByName(privconn-domains,
domain-name);
@@ -2476,13 +2483,6 @@ static int testDomainCoreDumpWithFormat(virDomainPtr
domain,
}
}
-/* dump the core of domain to file to */
-if (dumpformat != VIR_DUMP_FORMAT_RAW) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
- _(kdump-compressed format is not supported here));
-goto cleanup;
-}
-
ret = 0;
cleanup:
VIR_FORCE_CLOSE(fd);
@@ -2497,7 +2497,8 @@ cleanup:
static int testDomainCoreDump(virDomainPtr domain,
const char *to,
unsigned int flags) {
-return testDomainCoreDumpWithFormat(domain, to, VIR_DUMP_FORMAT_RAW,
flags);
+return testDomainCoreDumpWithFormat(domain, to,
+VIR_DOMAIN_CORE_DUMP_FORMAT_RAW,
flags);
}
static char *testDomainGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED) {
@@ -7350,6 +7351,7 @@ static virDriver testDriver = {
.domainRestore = testDomainRestore, /* 0.3.2 */
.domainRestoreFlags = testDomainRestoreFlags, /* 0.9.4 */
.domainCoreDump = testDomainCoreDump, /* 0.3.2 */
+.domainCoreDumpWithFormat = testDomainCoreDumpWithFormat, /* 1.2.3 */
.domainSetVcpus = testDomainSetVcpus, /* 0.1.4 */
.domainSetVcpusFlags = testDomainSetVcpusFlags, /* 0.8.5 */
.domainGetVcpusFlags = testDomainGetVcpusFlags, /* 0.8.5 */
--
Regards
Qiao Nuohan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hotplug:Fix log mistake in qemuMonitorAddNetdev
On Thu, Mar 13, 2014 at 09:26:16AM +, Wangrui (K) wrote:
From 81de0ce710bad91a2df18247e681b5a83872b8d5 Mon Sep 17 00:00:00 2001
From: Wang Rui moon.wang...@huawei.com
Date: Thu, 13 Mar 2014 17:05:03 +
Subject: [PATCH] hotplug:Fix log mistake in qemuMonitorAddNetdev
VIR_DEBUG in qemuMonitorAddNetdev should print vhostfdSize
Signed-off-by: Wang Rui moon.wang...@huawei.com
---
src/qemu/qemu_monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index e4707b7..b2af0ae 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2781,7 +2781,7 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon,
VIR_DEBUG(mon=%p netdevstr=%s tapfd=%p tapfdName=%p tapfdSize=%d
vhostfd=%p vhostfdName=%p vhostfdSize=%d,
mon, netdevstr, tapfd, tapfdName, tapfdSize,
- vhostfd, vhostfdName, tapfdSize);
+ vhostfd, vhostfdName, vhostfdSize);
if (!mon) {
virReportError(VIR_ERR_INVALID_ARG, %s,
--
1.8.3.4
ACK and pushed, thanks.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 1/3] add new virDomainCoreDumpWithFormat API
On Mon, Mar 17, 2014 at 09:37:46AM +, Daniel P. Berrange wrote: On Thu, Mar 13, 2014 at 08:42:40AM +0100, Martin Kletzander wrote: On Wed, Mar 12, 2014 at 09:29:55AM -0600, Eric Blake wrote: On 03/12/2014 09:04 AM, Martin Kletzander wrote: On Thu, Mar 06, 2014 at 09:35:47AM +, qiaonuo...@cn.fujitsu.com wrote: --memory-only option is introduced without compression supported. Therefore, this is a freature regression of virsh dump. Now qemu has support dumping memory s/freature/feature/ but I would not use the word regression since that never worked. Also it would help mentioning the commit ID or a version it got included in qemu. On that note, is there a possibility of of introspection of that feature, so we can gracefully error out in case older qemu is used? Yes - qemu 2.0 is adding 'query-dump-guest-memory-capability', which can be used for two purposes: 1. if this command exists, 'dump-guest-memory' supports the new optional 'format' argument; and 2. calling this command will return a list of WHICH formats are supported by the given qemu binary (since configure-time choices can disable some of the formats from actually working). So you need to have a patch that modifies src/qemu/qemu_capabilities.[ch] to do the probing and set capability bits, so that we can gracefully error out when talking to a too-old qemu, or requesting a format that was not compiled in to a new qemu. Great. Could we have the compression parameter just as an arbitrary string then (properly checked for, etc.) so there is no need to adapt our code to qemu format additions? I rather prefer it as an enum. Just doing a plain string passthrough from the API means that the API ends up exposing impl details of QEMU. This has caused us pain the past - eg nic model XML used to just be a free-form string, instead of an enum parsed string. The result was the same NIC model ended up with different names across different hypervisors. Using an enum / int is good for the API, even if it means we need to make changes if QEMU adds more formats That makes sense, thanks for the explanation. That's why I was asking. Martin Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix leak of iterator in nwfilter instantiate code
On Mon, Mar 17, 2014 at 11:40:39AM +, Daniel P. Berrange wrote:
The ebiptablesCreateRuleInstanceIterate creates a
virNWFilterVarCombIterPtr instance and iterates over
it. Unfortunately in doing so, it discards the original
pointer. At the end of the method it will thus effectively
do virNWFilterVarCombIterFree(NULL), which means it will
leak the iterator.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c
b/src/nwfilter/nwfilter_ebiptables_driver.c
index 57c0476..9dbd3ff 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -2865,14 +2865,14 @@ ebiptablesCreateRuleInstanceIterate(
virNWFilterRuleInstPtr res)
{
int rc = 0;
-virNWFilterVarCombIterPtr vciter;
+virNWFilterVarCombIterPtr vciter, tmp;
/* rule-vars holds all the variables names that this rule will access.
* iterate over all combinations of the variables' values and instantiate
* the filtering rule with each combination.
*/
-vciter = virNWFilterVarCombIterCreate(vars,
- rule-varAccess, rule-nVarAccess);
+tmp = vciter = virNWFilterVarCombIterCreate(vars,
+rule-varAccess,
rule-nVarAccess);
ACK with this line wrapped (too long).
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix leak of iterator in nwfilter instantiate code
On Mon, Mar 17, 2014 at 12:33:24PM +, Daniel P. Berrange wrote: On Mon, Mar 17, 2014 at 11:40:39AM +, Daniel P. Berrange wrote: The ebiptablesCreateRuleInstanceIterate creates a virNWFilterVarCombIterPtr instance and iterates over it. Unfortunately in doing so, it discards the original pointer. At the end of the method it will thus effectively do virNWFilterVarCombIterFree(NULL), which means it will leak the iterator. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) Opps, this is wrong. The virNWFilterVarCombIterNext method will bizarely free its input parameter at the end. I'll send a different patch to give this saner semantics. Haven't noticed it in the review, sorry, disregard that ACK then :( Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Give virNWFilterVarCombIterNext saner semantics
On Mon, Mar 17, 2014 at 12:34:21PM +, Daniel P. Berrange wrote:
The virNWFilterVarCombIterNext method will free its
parameter when it gets to the end of the iterator.
This is somewhat misleading design, making it appear
as if the caller has a memory leak. Remove the free'ing
of the parameter and ensure that the calling method
ebiptablesCreateRuleInstanceIterate free's it instead.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/conf/nwfilter_params.c| 4 +---
src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++--
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index a78c407..5393134 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -526,10 +526,8 @@ next:
}
}
-if (ci-nIter == i) {
-virNWFilterVarCombIterFree(ci);
+if (ci-nIter == i)
return NULL;
-}
return ci;
}
Adding this hunk is indeed needed, I spoke to soon the first time.
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c
b/src/nwfilter/nwfilter_ebiptables_driver.c
index 0505045..e1e0af8 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -2869,14 +2869,14 @@ ebiptablesCreateRuleInstanceIterate(
virNWFilterRuleInstPtr res)
{
int rc = 0;
-virNWFilterVarCombIterPtr vciter;
+virNWFilterVarCombIterPtr vciter, tmp;
/* rule-vars holds all the variables names that this rule will access.
* iterate over all combinations of the variables' values and instantiate
* the filtering rule with each combination.
*/
-vciter = virNWFilterVarCombIterCreate(vars,
- rule-varAccess, rule-nVarAccess);
+tmp = vciter = virNWFilterVarCombIterCreate(vars,
+rule-varAccess,
rule-nVarAccess);
But this line should be wrapped, still.
ACK to this version with the line wrapped, then.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] Explicitly cast some switch parameters to enum
This patch is not trying to fix every switch, just the ones I worked
with last time, because some of these were especially unreadable.
Covers enums virDomainGraphicsType and virDomainChrType (where
applicable).
Also sort it's cases by their value.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c | 22 +-
src/qemu/qemu_command.c | 4 +++-
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1bf1268..e566140 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1178,7 +1178,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
if (!def)
return;
-switch (def-type) {
+switch ((enum virDomainGraphicsType)def-type) {
case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
VIR_FREE(def-data.vnc.socket);
VIR_FREE(def-data.vnc.keymap);
@@ -1201,6 +1201,10 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr
def)
VIR_FREE(def-data.spice.keymap);
virDomainGraphicsAuthDefClear(def-data.spice.auth);
break;
+
+case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
+/* coverity[dead_error_begin] */
+break;
}
for (i = 0; i def-nListens; i++)
@@ -1575,7 +1579,7 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef
*src,
if (tgt-type != src-type)
return false;
-switch (src-type) {
+switch ((enum virDomainChrType)src-type) {
case VIR_DOMAIN_CHR_TYPE_PTY:
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
@@ -1604,16 +1608,15 @@ virDomainChrSourceDefIsEqual(const
virDomainChrSourceDef *src,
tgt-data.spiceport.channel);
break;
+case VIR_DOMAIN_CHR_TYPE_NULL:
case VIR_DOMAIN_CHR_TYPE_VC:
case VIR_DOMAIN_CHR_TYPE_STDIO:
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+case VIR_DOMAIN_CHR_TYPE_LAST:
/* nada */
-return true;
+break;
}
-/* This should happen only on new,
- * yet unhandled type */
-
return false;
}
@@ -7269,11 +7272,11 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr
def,
}
switch ((enum virDomainChrType) def-type) {
-case VIR_DOMAIN_CHR_TYPE_LAST:
case VIR_DOMAIN_CHR_TYPE_NULL:
+case VIR_DOMAIN_CHR_TYPE_VC:
case VIR_DOMAIN_CHR_TYPE_STDIO:
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
-case VIR_DOMAIN_CHR_TYPE_VC:
+case VIR_DOMAIN_CHR_TYPE_LAST:
break;
case VIR_DOMAIN_CHR_TYPE_PTY:
@@ -15724,11 +15727,12 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
}
virBufferAddLit(buf, \n);
-switch (def-type) {
+switch ((enum virDomainChrType)def-type) {
case VIR_DOMAIN_CHR_TYPE_NULL:
case VIR_DOMAIN_CHR_TYPE_VC:
case VIR_DOMAIN_CHR_TYPE_STDIO:
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+case VIR_DOMAIN_CHR_TYPE_LAST:
/* nada */
break;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 64fd748..d89c440 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6021,7 +6021,7 @@ qemuBuildChrArgStr(virDomainChrSourceDefPtr dev, const
char *prefix)
if (prefix)
virBufferAdd(buf, prefix, strlen(prefix));
-switch (dev-type) {
+switch ((enum virDomainChrType)dev-type) {
case VIR_DOMAIN_CHR_TYPE_NULL:
virBufferAddLit(buf, null);
break;
@@ -6089,7 +6089,9 @@ qemuBuildChrArgStr(virDomainChrSourceDefPtr dev, const
char *prefix)
dev-data.nix.listen ? ,server,nowait : );
break;
+case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+case VIR_DOMAIN_CHR_TYPE_LAST:
break;
}
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] qemu: agent availability cleanup
Eliminate all the code re-use which checks for priv-agentError or
priv-agent.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_domain.c | 22 +
src/qemu/qemu_domain.h | 4 ++
src/qemu/qemu_driver.c | 121 ++---
3 files changed, 40 insertions(+), 107 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bc0b8f7..4465bef 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2465,3 +2465,25 @@ cleanup:
virDomainDefFree(migratableDefDst);
return ret;
}
+
+bool
+qemuDomainAgentAvailable(qemuDomainObjPrivatePtr priv,
+ bool reportError)
+{
+if (priv-agentError) {
+if (reportError) {
+virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s,
+ _(QEMU guest agent is not
+ available due to an error));
+}
+return false;
+}
+if (!priv-agent) {
+if (reportError) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+ _(QEMU guest agent is not configured));
+}
+return false;
+}
+return true;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 0bed50b..b2830c4 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -376,4 +376,8 @@ int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
virDomainDefPtr src,
virDomainDefPtr dst);
+
+bool qemuDomainAgentAvailable(qemuDomainObjPrivatePtr priv,
+ bool reportError);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 89f443f..99a2840 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1798,6 +1798,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags) {
qemuDomainObjPrivatePtr priv;
bool useAgent = false, agentRequested, acpiRequested;
bool isReboot = false;
+bool agentForced;
int agentFlag = QEMU_AGENT_SHUTDOWN_POWERDOWN;
virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
@@ -1824,25 +1825,11 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags) {
if (virDomainShutdownFlagsEnsureACL(dom-conn, vm-def, flags) 0)
goto cleanup;
-if (priv-agentError) {
-if (agentRequested !acpiRequested) {
-virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s,
- _(QEMU guest agent is not
- available due to an error));
+agentForced = agentRequested !acpiRequested;
+if (!qemuDomainAgentAvailable(priv, agentForced)) {
+if (agentForced)
goto cleanup;
-} else {
-useAgent = false;
-}
-}
-
-if (!priv-agent) {
-if (agentRequested !acpiRequested) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
- _(QEMU guest agent is not configured));
-goto cleanup;
-} else {
-useAgent = false;
-}
+useAgent = false;
}
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
@@ -1930,18 +1917,8 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
priv-agent))
useAgent = true;
-if (useAgent) {
-if (priv-agentError) {
-virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s,
- _(QEMU guest agent is not
- available due to an error));
-goto cleanup;
-}
-if (!priv-agent) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
- _(QEMU guest agent is not configured));
-goto cleanup;
-}
+if (useAgent !qemuDomainAgentAvailable(priv, true)) {
+goto cleanup;
} else {
#if WITH_YAJL
if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON)) {
@@ -4187,18 +4164,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
goto endjob;
}
-if (priv-agentError) {
-virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s,
- _(QEMU guest agent is not
- available due to an error));
-goto endjob;
-}
-
-if (!priv-agent) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
- _(QEMU guest agent is not configured));
+if (!qemuDomainAgentAvailable(priv, true))
goto endjob;
-}
if (nvcpus vm-def-vcpus) {
virReportError(VIR_ERR_INVALID_ARG,
@@ -4925,18 +4892,8 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int
flags)
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) 0)
goto cleanup;
-if (priv
[libvirt] [PATCH 7/7] Require spaces around equality comparisons
Commit a1cbe4b5 added a check for spaces around assignments and this
patch extends it to checks for spaces around '=='. One exception is
virAssertCmpInt where comma after '==' is aceptable (since it is a
macro and '==' is its argument).
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
build-aux/bracket-spacing.pl | 9 ++---
src/libvirt.c | 2 +-
src/locking/lock_driver_sanlock.c | 4 ++--
src/qemu/qemu_command.c | 6 +++---
src/rpc/virnetclient.c| 10 +-
src/util/vircgroup.c | 12 ++--
src/util/virthreadpool.c | 3 ++-
src/vbox/vbox_tmpl.c | 2 +-
src/xenapi/xenapi_driver.c| 4 ++--
tests/commandtest.c | 2 +-
tests/domainsnapshotxml2xmltest.c | 2 +-
tests/interfacexml2xmltest.c | 2 +-
tests/libvirtdconftest.c | 4 ++--
tests/lxcxml2xmltest.c| 2 +-
tests/networkxml2conftest.c | 2 +-
tests/networkxml2xmltest.c| 2 +-
tests/nodedevxml2xmltest.c| 2 +-
tests/nodeinfotest.c | 2 +-
tests/nwfilterxml2xmltest.c | 2 +-
tests/qemuargv2xmltest.c | 2 +-
tests/qemuxml2argvtest.c | 2 +-
tests/qemuxml2xmltest.c | 2 +-
tests/qemuxmlnstest.c | 2 +-
tests/sexpr2xmltest.c | 2 +-
tests/sockettest.c| 4 ++--
tests/statstest.c | 2 +-
tests/storagepoolxml2xmltest.c| 2 +-
tests/storagevolxml2argvtest.c| 2 +-
tests/storagevolxml2xmltest.c | 2 +-
tests/virauthconfigtest.c | 4 ++--
tests/virbuftest.c| 2 +-
tests/vircgrouptest.c | 2 +-
tests/virdbustest.c | 4 ++--
tests/virdrivermoduletest.c | 4 ++--
tests/virhostdevtest.c| 2 +-
tests/viridentitytest.c | 4 ++--
tests/virkeyfiletest.c| 4 ++--
tests/virkmodtest.c | 2 +-
tests/virlockspacetest.c | 4 ++--
tests/virnetmessagetest.c | 4 ++--
tests/virnetsockettest.c | 2 +-
tests/virnettlscontexttest.c | 4 ++--
tests/virnettlssessiontest.c | 4 ++--
tests/virpcitest.c| 4 ++--
tests/virportallocatortest.c | 2 +-
tests/virshtest.c | 2 +-
tests/virstringtest.c | 5 ++---
tests/virsystemdtest.c| 4 ++--
tests/virtimetest.c | 4 ++--
tests/viruritest.c| 4 ++--
tests/xencapstest.c | 2 +-
tests/xmconfigtest.c | 4 ++--
tests/xml2sexprtest.c | 2 +-
tools/virsh-domain-monitor.c | 2 +-
tools/virsh-pool.c| 2 +-
55 files changed, 91 insertions(+), 88 deletions(-)
diff --git a/build-aux/bracket-spacing.pl b/build-aux/bracket-spacing.pl
index 4f9f13a..e4ae8f0 100755
--- a/build-aux/bracket-spacing.pl
+++ b/build-aux/bracket-spacing.pl
@@ -145,9 +145,12 @@ foreach my $file (@ARGV) {
last;
}
-# Require spaces around assignment '=' and compounds
-while ($data =~ /[^!|\-+*\/%\^'= ]=[^=]/ ||
- $data =~ /[^!|\-+*\/%\^'=]=[^= \\\n]/) {
+# Require spaces around assignment '=', compounds and '=='
+# with the exception of virAssertCmpInt()
+while ($data =~ /[^!|\-+*\/%\^'= ]=\+[^=]/ ||
+ $data =~ /[^!|\-+*\/%\^'=]=[^= \\\n]/ ||
+ $data =~ /[\S]==/ ||
+ ($data =~ /==[^\s,]/ $data !~ /[\s]virAssertCmpInt\(/)) {
print $file:$.: $line;
$ret = 1;
last;
diff --git a/src/libvirt.c b/src/libvirt.c
index a385935..6715fc6 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -9209,7 +9209,7 @@ error:
* Not all hypervisors will support sending signals to
* arbitrary processes or process groups. If this API is
* implemented the minimum requirement is to be able to
- * use @pid_value==1 (i.e. kill init). No other value is
+ * use @pid_value == 1 (i.e. kill init). No other value is
* required to be supported.
*
* If the @signum is VIR_DOMAIN_PROCESS_SIGNAL_NOP then this
diff --git a/src/locking/lock_driver_sanlock.c
b/src/locking/lock_driver_sanlock.c
index 0c87048..c54d755 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -1,7 +1,7 @@
/*
* lock_driver_sanlock.c: A lock driver for Sanlock
*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -912,7 +912,7 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr
lock,
/* We only initialize 'sock' if we are in the real
* child process and we need it to be inherited
*
- * If sock==-1, then sanlock auto-open/closes a
+ * If sock == -1, then sanlock auto-open/closes a
* temporary sock
[libvirt] [PATCH 4/7] Don't leave empty first line in C source files
If there should be some sort of separator it is better to use comment with the filename, copyright, description, license information and authors. Found by: git grep -nH '^$' | grep '\.[ch]:1:' Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: If any reviewer wants I can add a syntax-check for that. src/esx/esx_device_monitor.c | 1 - src/esx/esx_device_monitor.h | 1 - src/esx/esx_driver.c | 1 - src/esx/esx_driver.h | 1 - src/esx/esx_interface_driver.c | 1 - src/esx/esx_interface_driver.h | 1 - src/esx/esx_network_driver.c | 1 - src/esx/esx_network_driver.h | 1 - src/esx/esx_nwfilter_driver.c| 1 - src/esx/esx_nwfilter_driver.h| 1 - src/esx/esx_private.h| 1 - src/esx/esx_secret_driver.c | 1 - src/esx/esx_secret_driver.h | 1 - src/esx/esx_storage_backend_iscsi.c | 1 - src/esx/esx_storage_backend_iscsi.h | 1 - src/esx/esx_storage_backend_vmfs.c | 1 - src/esx/esx_storage_backend_vmfs.h | 1 - src/esx/esx_storage_driver.c | 1 - src/esx/esx_storage_driver.h | 1 - src/esx/esx_util.c | 1 - src/esx/esx_vi.c | 1 - src/esx/esx_vi.h | 1 - src/esx/esx_vi_methods.c | 1 - src/esx/esx_vi_methods.h | 1 - src/esx/esx_vi_types.c | 1 - src/esx/esx_vi_types.h | 1 - src/hyperv/hyperv_device_monitor.c | 1 - src/hyperv/hyperv_device_monitor.h | 1 - src/hyperv/hyperv_driver.c | 1 - src/hyperv/hyperv_driver.h | 1 - src/hyperv/hyperv_interface_driver.c | 1 - src/hyperv/hyperv_interface_driver.h | 1 - src/hyperv/hyperv_network_driver.c | 1 - src/hyperv/hyperv_network_driver.h | 1 - src/hyperv/hyperv_nwfilter_driver.c | 1 - src/hyperv/hyperv_nwfilter_driver.h | 1 - src/hyperv/hyperv_private.h | 1 - src/hyperv/hyperv_secret_driver.c| 1 - src/hyperv/hyperv_secret_driver.h| 1 - src/hyperv/hyperv_storage_driver.c | 1 - src/hyperv/hyperv_storage_driver.h | 1 - src/hyperv/hyperv_util.c | 1 - src/hyperv/hyperv_util.h | 1 - src/hyperv/hyperv_wmi.c | 1 - src/hyperv/hyperv_wmi.h | 1 - src/hyperv/hyperv_wmi_classes.c | 1 - src/hyperv/hyperv_wmi_classes.h | 1 - src/hyperv/openwsman.h | 1 - src/qemu/qemu_migration.c| 1 - src/security/virt-aa-helper.c| 1 - src/vbox/vbox_MSCOMGlue.c| 1 - src/vbox/vbox_MSCOMGlue.h| 1 - src/vbox/vbox_glue.c | 1 - src/vbox/vbox_glue.h | 1 - src/vmx/vmx.h| 1 - tests/testutilslxc.h | 1 - tests/testutilsxen.h | 1 - tests/xml2sexprtest.c| 1 - 58 files changed, 58 deletions(-) diff --git a/src/esx/esx_device_monitor.c b/src/esx/esx_device_monitor.c index 7dc0618..11b61c9 100644 --- a/src/esx/esx_device_monitor.c +++ b/src/esx/esx_device_monitor.c @@ -1,4 +1,3 @@ - /* * esx_device_monitor.c: device monitor functions for managing VMware ESX * host devices diff --git a/src/esx/esx_device_monitor.h b/src/esx/esx_device_monitor.h index a1efdcb..1b2795e 100644 --- a/src/esx/esx_device_monitor.h +++ b/src/esx/esx_device_monitor.h @@ -1,4 +1,3 @@ - /* * esx_device_monitor.h: device monitor methods for managing VMware ESX * host devices diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 6a2efe3..2512a6e 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1,4 +1,3 @@ - /* * esx_driver.c: core driver functions for managing VMware ESX hosts * diff --git a/src/esx/esx_driver.h b/src/esx/esx_driver.h index 9df348d..0de6a64 100644 --- a/src/esx/esx_driver.h +++ b/src/esx/esx_driver.h @@ -1,4 +1,3 @@ - /* * esx_driver.h: core driver functions for managing VMware ESX hosts * diff --git a/src/esx/esx_interface_driver.c b/src/esx/esx_interface_driver.c index dcb9f03..193565e 100644 --- a/src/esx/esx_interface_driver.c +++ b/src/esx/esx_interface_driver.c @@ -1,4 +1,3 @@ - /* * esx_interface_driver.c: interface driver functions for managing VMware ESX * host interfaces diff --git a/src/esx/esx_interface_driver.h b/src/esx/esx_interface_driver.h index 8cacc44..cf0c4bd 100644 --- a/src/esx/esx_interface_driver.h +++ b/src/esx/esx_interface_driver.h @@ -1,4 +1,3 @@ - /* * esx_interface_driver.h: interface driver functions for managing VMware ESX * host interfaces diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index c8b53b1..4449ced 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -1,4 +1,3 @@ - /* * esx_network_driver.c: network driver functions for managing VMware ESX * host networks diff --git a/src/esx
[libvirt] [PATCH 2/7] tests: cleanup object-locking test
When ran, cil is throwing out some errors and warnings for obsolete
'or' unused variables and wrong module name (it should not contain a
hyphen; hence the rename).
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
I still can't run it successfully, but no matter what we'll do with
it, there is no downside of having the code clean.
.gitignore| 6 +++---
tests/Makefile.am | 12 ++--
tests/{object-locking.ml = objectlocking.ml} | 10 --
3 files changed, 13 insertions(+), 15 deletions(-)
rename tests/{object-locking.ml = objectlocking.ml} (98%)
diff --git a/.gitignore b/.gitignore
index 027c203..0513a33 100644
--- a/.gitignore
+++ b/.gitignore
@@ -149,9 +149,9 @@
/tests/*test
!/tests/*schematest
!/tests/virt-aa-helper-test
-/tests/object-locking
-/tests/object-locking-files.txt
-/tests/object-locking.cm[ix]
+/tests/objectlocking
+/tests/objectlocking-files.txt
+/tests/objectlocking.cm[ix]
/tests/reconnect
/tests/ssh
/tests/test_conf
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1745469..f80e7ad 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -228,7 +228,7 @@ test_programs += vmwarevertest
endif WITH_VMWARE
if WITH_CIL
-test_programs += object-locking
+test_programs += objectlocking
endif WITH_CIL
if WITH_YAJL
@@ -1016,21 +1016,21 @@ CILOPTINCS =
CILOPTPACKAGES = -package unix,str,cil
CILOPTLIBS = -linkpkg
-object_locking_SOURCES = object-locking.ml
+object_locking_SOURCES = objectlocking.ml
%.cmx: %.ml
ocamlfind ocamlopt $(CILOPTFLAGS) $(CILOPTINCS) $(CILOPTPACKAGES) -c $
-object-locking: object-locking.cmx object-locking-files.txt
+objectlocking: objectlocking.cmx objectlocking-files.txt
ocamlfind ocamlopt $(CILOPTFLAGS) $(CILOPTINCS) $(CILOPTPACKAGES) \
$(CILOPTLIBS) $ -o $@
-object-locking-files.txt:
+objectlocking-files.txt:
find $(top_builddir)/src/ -name '*.i' $@
else ! WITH_CIL
-EXTRA_DIST += object-locking.ml
+EXTRA_DIST += objectlocking.ml
endif ! WITH_CIL
CLEANFILES = *.cov *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda *.cmi *.cmx \
- object-locking-files.txt
+ objectlocking-files.txt
diff --git a/tests/object-locking.ml b/tests/objectlocking.ml
similarity index 98%
rename from tests/object-locking.ml
rename to tests/objectlocking.ml
index 009b8f8..2476731 100644
--- a/tests/object-locking.ml
+++ b/tests/objectlocking.ml
@@ -1,7 +1,7 @@
(*
* Analyse libvirt driver API methods for mutex locking mistakes
*
- * Copyright (C) 2008-2010, 2012 Red Hat, Inc.
+ * Copyright (C) 2008-2010, 2012, 2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -623,7 +623,7 @@ module L = DF.ForwardsDataFlow(Locking)
let () =
(* Read the list of files from libvirt-files. *)
- let files = input_file object-locking-files.txt in
+ let files = input_file objectlocking-files.txt in
(* Load parse each input file. *)
let files =
@@ -645,7 +645,6 @@ let () =
let driverVars = List.filter (
function
| GVar (varinfo, initinfo, loc) - (* global variable *)
- let name = varinfo.vname in
if isDriverTable varinfo then
true
else
@@ -656,7 +655,6 @@ let () =
let driverVarFuncs = List.map (
function
| GVar (varinfo, initinfo, loc) - (* global variable *)
- let name = varinfo.vname in
if isDriverTable varinfo then
getDriverFuncNames initinfo
else
@@ -752,7 +750,7 @@ let () =
IH.find Locking.stmtStartData st.sid in
let leakDrivers = not (VS.is_empty ld) in
let leakObjects = not (VS.is_empty lo) in
- leakDrivers or leakObjects
+ leakDrivers || leakObjects
) exitPoints in
let mistakes = List.filter (
@@ -767,7 +765,7 @@ let () =
let deadLocked = (List.length dead) 0 in
- lockDriverOrdering or lockObjectOrdering or
useDriverUnlocked or useObjectUnlocked or deadLocked
+ lockDriverOrdering || lockObjectOrdering ||
useDriverUnlocked || useObjectUnlocked || deadLocked
) fundec.sallstmts in
if (List.length leaks) 0 || (List.length mistakes) 0 then (
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/7] Remove duplicate network model characters
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e566140..2a7d78f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6495,7 +6495,7 @@ error: } #define NET_MODEL_CHARS \ -abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_- +abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_- /* Parse the XML definition for a network interface * @param node XML nodeset to parse for net definition -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: fix problems with SRV
On Tue, Mar 18, 2014 at 12:07:17AM -0600, Laine Stump wrote:
A patch submitted by Steven Malin last week pointed out a problem with
libvirt's DNS SRV record configuration:
https://www.redhat.com/archives/libvir-list/2014-March/msg00536.html
When searching for that message later, I found another series that had
been posted by Guannan Ren back in 2012 that somehow slipped between
the cracks:
https://www.redhat.com/archives/libvir-list/2012-July/msg00236.html
That patch was very much out of date, but also pointed out some real
problems.
This patch fixes all the noted problems by refactoring
virNetworkDNSSrvDefParseXML() and networkDnsmasqConfContents(), then
verifies those fixes by added several new records to the test case.
Problems fixed:
* both service and protocol now have an underscore (_) prepended on
the commandline, as required by RFC2782.
srv service='sip' protocol='udp' domain='example.com'
target='tests.example.com' port='5060' priority='10'
weight='150'/
before: srv-host=sip.udp.example.com,tests.example.com,5060,10,150
after: srv-host=_sip._udp.example.com,tests.example.com,5060,10,150
* if domain wasn't specified in the srv element, the extra
trailing . will no longer be added to the dnsmasq commandline.
srv service='sip' protocol='udp' target='tests.example.com'
port='5060' priority='10' weight='150'/
before: srv-host=sip.udp.,tests.example.com,5060,10,150
after: srv-host=_sip._udp,tests.example.com,5060,10,150
* when optional attributes aren't specified, the separating comma is
also now not placed on the dnsmasq commandline. If optional
attributes in the middle of the line are not specified, they are
replaced with 0 in the commandline.
srv service='sip' protocol='udp' target='tests.example.com'
port='5060'/
before: srv-host=sip.udp.,tests.example.com,5060,,
after: srv-host=_sip._udp,tests.example.com,5060
(actually this would have generated an error, because optional
attributes weren't really optional).
* As a safeguard, the parser checks for a leading _ on service and
protocol, and fails if it is found. (Note that we can be guaranteed
that no existing valid configuration will have this, since until
now the parser had only allowed tcp or udp for protocol, and
the bridge driver wasn't prepending _, making it a 100% certainty
that there was no example of working SRV record use in the wild).
That's valid for protocol, but not for service. For service there was
a check for length only and it was not escaped at all. Even though
there couldn't be any abuse for that, settings that resulted in
generating invalid config file for dnsmasq were parsed and saved
without any error from libvirt.
* the domain attribute is no longer required in order to recognize
the port, priority, and weight attributes. Only target is required
for this.
* if target isn't specified, port, priority, and weight are not
allowed (since they are meaningless - an empty target means this
service is *not available* for this domain).
* port, priority, and weight are now truly optional, as the comments
originally suggested, but which was not actually true.
---
src/conf/network_conf.c| 113
+
src/network/bridge_driver.c| 75 --
.../nat-network-dns-srv-record-minimal.conf| 2 +-
.../nat-network-dns-srv-record.conf| 8 +-
.../nat-network-dns-srv-record.xml | 8 +-
5 files changed, 128 insertions(+), 78 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 9be06d3..3a28ac7 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -931,80 +931,107 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
virNetworkDNSSrvDefPtr def,
bool partialOkay)
{
+int ret;
+xmlNodePtr save_ctxt = ctxt-node;
+
+ctxt-node = node;
+
if (!(def-service = virXMLPropString(node, service)) !partialOkay)
{
virReportError(VIR_ERR_XML_DETAIL,
_(Missing required service attribute in DNS SRV
record
of network %s), networkName);
goto error;
}
-if (def-service strlen(def-service) DNS_RECORD_LENGTH_SRV) {
-virReportError(VIR_ERR_XML_DETAIL,
- _(Service name '%s' in network %s is too long, limit
is %d bytes),
- def-service, networkName, DNS_RECORD_LENGTH_SRV);
-goto error;
+if (def-service) {
+if (strlen(def-service) DNS_RECORD_LENGTH_SRV) {
+virReportError(VIR_ERR_XML_DETAIL,
+ _(service attribute '%s' in network %s is too
long,
+ limit is %d bytes),
+
Re: [libvirt] [PATCH v2 2/3] virsh: Prohibit virConnectOpen* functions in virsh
On Tue, Mar 18, 2014 at 08:05:51AM +0100, Michal Privoznik wrote:
On 10.03.2014 12:26, Martin Kletzander wrote:
Addition of vshConnect() makes virConnectOpen() functions obsolete in
virsh. Thus all virsh-*.[ch] files should be left only with
vshConnect() in the case of need.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
cfg.mk | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/cfg.mk b/cfg.mk
index 2a8957a..25446ac 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1,5 +1,5 @@
# Customize Makefile.maint. -*- makefile -*-
-# Copyright (C) 2008-2013 Red Hat, Inc.
+# Copyright (C) 2008-2014 Red Hat, Inc.
# Copyright (C) 2003-2008 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
@@ -863,6 +863,12 @@ sc_prohibit_atoi:
halt='Use virStrToLong* instead of atoi, atol, atof, atoq, atoll' \
$(_sc_search_regexp)
+sc_prohibit_virConnectOpen_in_virsh:
+ @prohibit='\bvirConnectOpen[a-zA-Z]* *\(' \
+ in_vc_files='^tools/virsh-.*\.[ch]$$'\
+ halt='Use vshConnect() in virsh instead of virConnectOpen*' \
+ $(_sc_search_regexp)
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
ACK
Michal
Pushed, thanks.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] virsh: Add keepalive in new vshConnect function
On Tue, Mar 18, 2014 at 08:05:54AM +0100, Michal Privoznik wrote: On 10.03.2014 12:26, Martin Kletzander wrote: Introducing keepalive similarly to Guannan around 2 years ago. Since we want to introduce keepalive for every connection, it makes sense to wrap the connecting function into new virsh one that can deal keepalive as well. Function vshConnect() is now used for connecting and keepalive added in that function (if possible) helps preventing long waits e.g. while nework goes down during migration. This patch also adds the options for keepalive tuning into virsh and fails connecting only when keepalives are explicitly requested and cannot be set (whether it is due to missing support in connected driver or remote server). If not explicitely requested, a debug message is printed (hence the addition to virsh-optparse test). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1073506 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=822839 Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: v2: - Skip calling virConnectSetKeepAlive() when keepalive-inteval is set to 0 - Add keepalive-related options into virsh man page - Just disable keepalive in virsh-optparse instead of checking for the error. tests/virsh-optparse | 4 +-- tools/virsh-domain.c | 2 +- tools/virsh.c| 81 +++- tools/virsh.h| 5 tools/virsh.pod | 12 5 files changed, 94 insertions(+), 10 deletions(-) ACK Michal Pushed, thanks. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix memory leak in virDomainChrSourceDefClear()
On Tue, Mar 18, 2014 at 01:02:24PM +0530, Nehal J Wani wrote: While running qemuxml2xmltest, it was found that valgrind pointed out the following memory leak: ==21905== 26 bytes in 1 blocks are definitely lost in loss record 23 of 69 ==21905==at 0x4A069EE: malloc (vg_replace_malloc.c:270) ==21905==by 0x3E782A754D: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6) ==21905==by 0x4CD986D: virDomainChrSourceDefParseXML (domain_conf.c:7233) ==21905==by 0x4CE4199: virDomainChrDefParseXML (domain_conf.c:7512) ==21905==by 0x4CFAF3F: virDomainDefParseXML (domain_conf.c:12303) ==21905==by 0x4CFB46E: virDomainDefParseNode (domain_conf.c:13031) ==21905==by 0x4CFB5E9: virDomainDefParse (domain_conf.c:12973) ==21905==by 0x41E9D8: testCompareXMLToXMLFiles (qemuxml2xmltest.c:40) ==21905==by 0x41EBAA: testCompareXMLToXMLHelper (qemuxml2xmltest.c:93) ==21905==by 0x421D21: virtTestRun (testutils.c:199) ==21905==by 0x41FCE9: mymain.part.0 (qemuxml2xmltest.c:244) ==21905==by 0x42249D: virtTestMain (testutils.c:782) ==21905== ... and 7 more --- The leaks were specific to the tests: DO_TEST(serial-spiceport); DO_TEST(serial-spiceport-nospice); Tanks for catching that. ACK and pushed now. Martin src/conf/domain_conf.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 92b1324..f633db7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1495,6 +1495,10 @@ virDomainChrSourceDefClear(virDomainChrSourceDefPtr def) case VIR_DOMAIN_CHR_TYPE_UNIX: VIR_FREE(def-data.nix.path); break; + +case VIR_DOMAIN_CHR_TYPE_SPICEPORT: +VIR_FREE(def-data.spiceport.channel); +break; } } -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/7] Fix inconsistency in using curly braces around functions
On Mon, Mar 17, 2014 at 09:25:13AM -0600, Eric Blake wrote:
On 03/17/2014 08:39 AM, Martin Kletzander wrote:
Although not explicitly requested, we are using KR (or Kernel)
indentation for curly braces around functions in HACKING file and most
of the code. The rest is inconsistent and this patch is trying to fix
the most of it.
Found by:
git grep -nH -e '^\s*\*\?[_a-zA-Z0-9]\+\(,\? \*\?[_a-zA-Z0-9]\+\)\+) \?{$' \
-e '^\s*[_a-zA-Z0-9]\+\( [_a-zA-Z0-9]\+\)*(\*\?[_a-zA-Z0-9]\+\(,\?
\*\?[_a-zA-Z0-9]\+\)\+) \?{$
and skipped foreach constructs which were found as well.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
This one's big. I'm reluctant to ack as-is; I think it could use two
things: first, can you split it into a series of smaller patches
(convert one directory or so at a time); second, add a cfg.mk check to
enforce the style, so outliers don't sneak back in.
I'll send a v2 split into smaller patches and I'll wrap long lines
too, no problem with that, but...
I was trying to tune the regexp to achieve 0 false positives and we
would need to use PCRE regexp which I don't know whether it's
supported by the sc_prohibit_ syntax checks.
The resulting regexps (or rather the whole command) look like this
(sorry for the long line):
git grep -nHP -e '^\s*\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9]+)+\) ?\{' -e
'^\s*(?!([a-zA-Z_]*for_?each[a-zA-Z_]*) ?\()[_a-zA-Z0-9]+( [_a-zA-Z0-9]+)*
?\(\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9\[\]]+)+\) ?\{'
or (listing all allowed foreach methods:
git grep -nHP -e '^\s*\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9]+)+\) ?\{' -e
'^\s*(?!(libxl_for_each_set_bit|udev_list_entry_foreach|nla_for_each_nested)
?\()[_a-zA-Z0-9]+( [_a-zA-Z0-9]+)* ?\(\*?[_a-zA-Z0-9]+(,?
\*?[_a-zA-Z0-9\[\]]+)+\) ?\{'
Let me know if we can somehow incorporate it inside cfg.mk, I'll
gladly do that. If not, bracket-spacing.pl will probably do the
trick, but anyway, we have to filter to *.[hc] files only.
Martin
+++ b/daemon/libvirtd-config.c
@@ -156,7 +156,8 @@ checkType(virConfValuePtr p, const char *filename,
} while (0)
-static int remoteConfigGetAuth(virConfPtr conf, const char *key, int
*auth, const char *filename) {
+static int remoteConfigGetAuth(virConfPtr conf, const char *key, int
*auth, const char *filename)
Hmm, this line is still longer than 80 columns. While touching this,
should we also do:
static int
remoteConfigGetAuth(virConfPtr conf,
const char *key,
int *auth,
const char *filename)
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-python 1/2] setPyVirTypedParameter: Copy full field name
On Tue, Mar 18, 2014 at 09:26:08AM +0100, Michal Privoznik wrote: In the setPyVirTypedParameter we try to produce virTypedParameter array from a pyhthon dictionary. However, when copying field name into s/pyhthon/python/ item in returned array, we use strncpy() as the field name is fixed length array. To determine its size we use sizeof() but mistakenly dereference it resulting in sizeof(char) which equals to 1 byte. Moreover, there's no need for using sizeof() when we have a global macro to tell us the length of the field name: VIR_TYPED_PARAM_FIELD_LENGTH. And since array is allocated using VIR_ALLOC() we are sure the memory is initially filled with zeros. Hence, there's no need to terminate string we've just copied into field name with '\0' character. It's there for sure too as we copy up to field length - 1. Signed-off-by: Michal Privoznik mpriv...@redhat.com ACK. Martin --- libvirt-override.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libvirt-override.c b/libvirt-override.c index 83369fc..3765a43 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -197,8 +197,7 @@ setPyVirTypedParameter(PyObject *info, goto cleanup; } -strncpy(temp-field, keystr, sizeof(*temp-field) - 1); -temp-field[sizeof(*temp-field) - 1] = '\0'; +strncpy(temp-field, keystr, VIR_TYPED_PARAM_FIELD_LENGTH - 1); temp-type = params[i].type; VIR_FREE(keystr); -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-python 2/2] setPyVirTypedParameter: free whole return variable on error
On Tue, Mar 18, 2014 at 09:26:09AM +0100, Michal Privoznik wrote: The @ret value is built in a loop. However, if in one iteration there's an error, we should free all the fields built so far. For instance, if there's an error and the previous item was type of VIR_TYPED_PARAM_STRING we definitely must free it. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- libvirt-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK, Martin diff --git a/libvirt-override.c b/libvirt-override.c index 3765a43..7f746ed 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -259,7 +259,7 @@ setPyVirTypedParameter(PyObject *info, return ret; cleanup: -VIR_FREE(ret); +virTypedParamsFree(ret, size); return NULL; } -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] qemu: agent availability cleanup
On Mon, Mar 17, 2014 at 09:16:07AM -0600, Eric Blake wrote: On 03/17/2014 08:39 AM, Martin Kletzander wrote: Eliminate all the code re-use which checks for priv-agentError or priv-agent. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_domain.c | 22 + src/qemu/qemu_domain.h | 4 ++ src/qemu/qemu_driver.c | 121 ++--- 3 files changed, 40 insertions(+), 107 deletions(-) Nice reduction in size. Thanks for the review and the ACK on IRC :) Pushed now, Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] tests: cleanup object-locking test
On Mon, Mar 17, 2014 at 09:19:02AM -0600, Eric Blake wrote:
On 03/17/2014 08:39 AM, Martin Kletzander wrote:
When ran, cil is throwing out some errors and warnings for obsolete
'or' unused variables and wrong module name (it should not contain a
hyphen; hence the rename).
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
I still can't run it successfully, but no matter what we'll do with
it, there is no downside of having the code clean.
.gitignore| 6 +++---
tests/Makefile.am | 12 ++--
tests/{object-locking.ml = objectlocking.ml} | 10 --
3 files changed, 13 insertions(+), 15 deletions(-)
rename tests/{object-locking.ml = objectlocking.ml} (98%)
ACK
Thanks, pushed.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/7] Don't leave empty first line in C source files
On Mon, Mar 17, 2014 at 09:27:14AM -0600, Eric Blake wrote:
On 03/17/2014 08:39 AM, Martin Kletzander wrote:
If there should be some sort of separator it is better to use comment
with the filename, copyright, description, license information and
authors.
Found by:
git grep -nH '^$' | grep '\.[ch]:1:'
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
If any reviewer wants I can add a syntax-check for that.
Yes, that would be a nice followup. We already have a check for
trailing blank lines; can that check be extended to also cover leading
blank lines?
That's a gnulib test which might be extended, but in their codebase,
not ours. There are few approaches that just emerged on my mind.
Since we want this to be enforced in .h files as well, we either have
to modify bracket-spacing.pl to skip .h files and feed those to it or
create another make target for this. The most readable and
straight-forward one is probably:
empty-lines-check:
$(AM_V_GEN)files=`$(VC_LIST) | grep '\.[hc]$$'`; \
grep -nH -m1 '^$$' $$files | grep ':1:$$' \
{ echo '$(ME): prohibited empty first line' 12; \
exit 1; }
the '-m1' is there to speed it up, unfortunately grep doesn't have a
parameter to scan only X lines of each file. We can use way more
tools than this and make it better, I just don't have a preference.
Can you give me a hint what would be the best to use from e.g. awk,
perl, just shell? Or whether we want to split this into
self-contained checking script like bracket-spacing.pl?
+++ b/tests/xml2sexprtest.c
@@ -1,4 +1,3 @@
-
#include config.h
ACK
Pushed it without the follow up for now. Thanks for the review.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/7] Remove duplicate network model characters
On Mon, Mar 17, 2014 at 09:40:46AM -0600, Eric Blake wrote: On 03/17/2014 08:39 AM, Martin Kletzander wrote: Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK Pushed, thanks. Martin diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e566140..2a7d78f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6495,7 +6495,7 @@ error: } #define NET_MODEL_CHARS \ -abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_- +abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_- /* Parse the XML definition for a network interface * @param node XML nodeset to parse for net definition -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/7] Explicitly cast some switch parameters to enum
On Tue, Mar 18, 2014 at 11:15:01AM +0100, Pavel Hrdina wrote: On 17.3.2014 16:38, Eric Blake wrote: On 03/17/2014 08:39 AM, Martin Kletzander wrote: This patch is not trying to fix every switch, just the ones I worked with last time, because some of these were especially unreadable. Covers enums virDomainGraphicsType and virDomainChrType (where applicable). Also sort it's cases by their value. s/it's/its/ (remember, it's is usable only if you could say it is and still make sense; in all other usage you want its) Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/conf/domain_conf.c | 22 +- src/qemu/qemu_command.c | 4 +++- 2 files changed, 16 insertions(+), 10 deletions(-) @@ -1201,6 +1201,10 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) VIR_FREE(def-data.spice.keymap); virDomainGraphicsAuthDefClear(def-data.spice.auth); break; + +case VIR_DOMAIN_GRAPHICS_TYPE_LAST: +/* coverity[dead_error_begin] */ +break; This Coverity comment is probably not necessary (but John can confirm faster than me). I've tested it and Coverity is happy also without the comment. ACK Pavel ACK once we figure out if we can drop that comment. Thank you both, I removed that comment and pushed it. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/7] Require spaces around equality comparisons
On Mon, Mar 17, 2014 at 09:44:45AM -0600, Eric Blake wrote: On 03/17/2014 08:39 AM, Martin Kletzander wrote: Commit a1cbe4b5 added a check for spaces around assignments and this patch extends it to checks for spaces around '=='. One exception is virAssertCmpInt where comma after '==' is aceptable (since it is a s/aceptable/acceptable/ macro and '==' is its argument). Signed-off-by: Martin Kletzander mklet...@redhat.com --- build-aux/bracket-spacing.pl | 9 ++--- Good, we enforce it for all future users. ACK Fixed and pushed, thanks. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] cfg.mk: Fix whitespaces
Just to align the backslashes with most of the file.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
Pushed as 'trivial'.
cfg.mk | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index e75323e..319210b 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -893,9 +893,9 @@ sc_prohibit_wrong_filename_in_comment:
fi;
sc_prohibit_virConnectOpen_in_virsh:
- @prohibit='\bvirConnectOpen[a-zA-Z]* *\(' \
- in_vc_files='^tools/virsh-.*\.[ch]$$'\
- halt='Use vshConnect() in virsh instead of virConnectOpen*' \
+ @prohibit='\bvirConnectOpen[a-zA-Z]* *\(' \
+ in_vc_files='^tools/virsh-.*\.[ch]$$' \
+ halt='Use vshConnect() in virsh instead of virConnectOpen*'\
$(_sc_search_regexp)
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 13/16] Use KR style for curly braces in src/network/bridge_driver.c
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/network/bridge_driver.c | 54 ++---
1 file changed, 36 insertions(+), 18 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 59b6c09..20930f3 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -380,7 +380,8 @@ networkFindActiveConfigs(virNetworkDriverStatePtr driver)
static void
-networkAutostartConfigs(virNetworkDriverStatePtr driver) {
+networkAutostartConfigs(virNetworkDriverStatePtr driver)
+{
size_t i;
for (i = 0; i driver-networks.count; i++) {
@@ -398,7 +399,8 @@ networkAutostartConfigs(virNetworkDriverStatePtr driver) {
#if HAVE_FIREWALLD
static DBusHandlerResult
firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED,
- DBusMessage *message, void *user_data) {
+ DBusMessage *message, void *user_data)
+{
virNetworkDriverStatePtr _driverState = user_data;
if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS,
@@ -567,7 +569,8 @@ networkStateAutoStart(void)
* files and update its state and the networking
*/
static int
-networkStateReload(void) {
+networkStateReload(void)
+{
if (!driverState)
return 0;
@@ -591,7 +594,8 @@ networkStateReload(void) {
* Shutdown the QEmu daemon, it will stop all active domains and networks
*/
static int
-networkStateCleanup(void) {
+networkStateCleanup(void)
+{
if (!driverState)
return -1;
@@ -2173,7 +2177,8 @@ static int
networkShutdownNetwork(virNetworkDriverStatePtr driver,
static virNetworkPtr networkLookupByUUID(virConnectPtr conn,
- const unsigned char *uuid) {
+ const unsigned char *uuid)
+{
virNetworkDriverStatePtr driver = conn-networkPrivateData;
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
@@ -2199,7 +2204,8 @@ cleanup:
}
static virNetworkPtr networkLookupByName(virConnectPtr conn,
- const char *name) {
+ const char *name)
+{
virNetworkDriverStatePtr driver = conn-networkPrivateData;
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
@@ -2237,12 +2243,14 @@ static virDrvOpenStatus networkOpen(virConnectPtr conn,
return VIR_DRV_OPEN_SUCCESS;
}
-static int networkClose(virConnectPtr conn) {
+static int networkClose(virConnectPtr conn)
+{
conn-networkPrivateData = NULL;
return 0;
}
-static int networkConnectNumOfNetworks(virConnectPtr conn) {
+static int networkConnectNumOfNetworks(virConnectPtr conn)
+{
int nactive = 0;
size_t i;
virNetworkDriverStatePtr driver = conn-networkPrivateData;
@@ -2297,7 +2305,8 @@ static int networkConnectListNetworks(virConnectPtr conn,
char **const names, in
return -1;
}
-static int networkConnectNumOfDefinedNetworks(virConnectPtr conn) {
+static int networkConnectNumOfDefinedNetworks(virConnectPtr conn)
+{
int ninactive = 0;
size_t i;
virNetworkDriverStatePtr driver = conn-networkPrivateData;
@@ -2623,7 +2632,8 @@ networkValidate(virNetworkDriverStatePtr driver,
return 0;
}
-static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) {
+static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml)
+{
virNetworkDriverStatePtr driver = conn-networkPrivateData;
virNetworkDefPtr def;
virNetworkObjPtr network = NULL;
@@ -2673,7 +2683,8 @@ cleanup:
return ret;
}
-static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) {
+static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml)
+{
virNetworkDriverStatePtr driver = conn-networkPrivateData;
virNetworkDefPtr def = NULL;
bool freeDef = true;
@@ -2738,7 +2749,8 @@ cleanup:
}
static int
-networkUndefine(virNetworkPtr net) {
+networkUndefine(virNetworkPtr net)
+{
virNetworkDriverStatePtr driver = net-conn-networkPrivateData;
virNetworkObjPtr network;
int ret = -1;
@@ -2969,7 +2981,8 @@ cleanup:
return ret;
}
-static int networkCreate(virNetworkPtr net) {
+static int networkCreate(virNetworkPtr net)
+{
virNetworkDriverStatePtr driver = net-conn-networkPrivateData;
virNetworkObjPtr network;
int ret = -1;
@@ -3003,7 +3016,8 @@ cleanup:
return ret;
}
-static int networkDestroy(virNetworkPtr net) {
+static int networkDestroy(virNetworkPtr net)
+{
virNetworkDriverStatePtr driver = net-conn-networkPrivateData;
virNetworkObjPtr network;
int ret = -1;
@@ -3107,7 +3121,8 @@ cleanup:
}
static int networkGetAutostart(virNetworkPtr net,
- int *autostart) {
+ int *autostart)
+{
virNetworkObjPtr network;
int ret = -1;
@@ -3127,7 +3142,8 @@ cleanup:
}
static int networkSetAutostart(virNetworkPtr net
[libvirt] [PATCH v2 01/16] Use KR style for curly braces in tests/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
tests/commandhelper.c| 5 +++--
tests/qemuargv2xmltest.c | 3 ++-
tests/shunloadhelper.c | 5 +++--
tests/testutils.c| 15 +-
tests/testutilslxc.c | 3 ++-
tests/testutilsqemu.c| 3 ++-
tests/testutilsxen.c | 3 ++-
tests/virshtest.c| 54
tests/virusbtest.c | 3 ++-
tests/xencapstest.c | 33 +++--
10 files changed, 84 insertions(+), 43 deletions(-)
diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index 296fbbb..32b8512 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -1,7 +1,7 @@
/*
* commandhelper.c: Auxiliary program for commandtest
*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -37,7 +37,8 @@
# define VIR_FROM_THIS VIR_FROM_NONE
-static int envsort(const void *a, const void *b) {
+static int envsort(const void *a, const void *b)
+{
const char *const*astrptr = a;
const char *const*bstrptr = b;
const char *astr = *astrptr;
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 4923c2b..9eb3c49 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -36,7 +36,8 @@ static int blankProblemElements(char *data)
static int testCompareXMLToArgvFiles(const char *xml,
const char *cmdfile,
- bool expect_warning) {
+ bool expect_warning)
+{
char *expectxml = NULL;
char *actualxml = NULL;
char *cmd = NULL;
diff --git a/tests/shunloadhelper.c b/tests/shunloadhelper.c
index f2afbe8..128adce 100644
--- a/tests/shunloadhelper.c
+++ b/tests/shunloadhelper.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011 Red Hat, Inc.
+ * Copyright (C) 2011, 2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -38,7 +38,8 @@ static void shunloadError(void *userData ATTRIBUTE_UNUSED,
int shunloadStart(void);
-int shunloadStart(void) {
+int shunloadStart(void)
+{
virConnectPtr conn;
virSetErrorFunc(NULL, shunloadError);
diff --git a/tests/testutils.c b/tests/testutils.c
index e21e2f4..179c72a 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -362,7 +362,8 @@ virtTestLoadFile(const char *file, char **buf)
#ifndef WIN32
static
void virtTestCaptureProgramExecChild(const char *const argv[],
- int pipefd) {
+ int pipefd)
+{
size_t i;
int open_max;
int stdinfd = -1;
@@ -629,7 +630,8 @@ virtTestLogContentAndReset(void)
static unsigned int
-virTestGetFlag(const char *name) {
+virTestGetFlag(const char *name)
+{
char *flagStr;
unsigned int flag;
@@ -643,21 +645,24 @@ virTestGetFlag(const char *name) {
}
unsigned int
-virTestGetDebug(void) {
+virTestGetDebug(void)
+{
if (testDebug == -1)
testDebug = virTestGetFlag(VIR_TEST_DEBUG);
return testDebug;
}
unsigned int
-virTestGetVerbose(void) {
+virTestGetVerbose(void)
+{
if (testVerbose == -1)
testVerbose = virTestGetFlag(VIR_TEST_VERBOSE);
return testVerbose || virTestGetDebug();
}
unsigned int
-virTestGetExpensive(void) {
+virTestGetExpensive(void)
+{
if (testExpensive == -1)
testExpensive = virTestGetFlag(VIR_TEST_EXPENSIVE);
return testExpensive;
diff --git a/tests/testutilslxc.c b/tests/testutilslxc.c
index 1bc6feb..a1574d3 100644
--- a/tests/testutilslxc.c
+++ b/tests/testutilslxc.c
@@ -8,7 +8,8 @@
# include domain_conf.h
-virCapsPtr testLXCCapsInit(void) {
+virCapsPtr testLXCCapsInit(void)
+{
virCapsPtr caps;
virCapsGuestPtr guest;
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 726501c..f7810f6 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -203,7 +203,8 @@ error:
return -1;
}
-virCapsPtr testQemuCapsInit(void) {
+virCapsPtr testQemuCapsInit(void)
+{
virCapsPtr caps;
virCapsGuestPtr guest;
virCapsGuestMachinePtr *machines = NULL;
diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
index 1b5ee79..f3216e9 100644
--- a/tests/testutilsxen.c
+++ b/tests/testutilsxen.c
@@ -7,7 +7,8 @@
#include domain_conf.h
-virCapsPtr testXenCapsInit(void) {
+virCapsPtr testXenCapsInit(void)
+{
struct utsname utsname;
virCapsPtr caps;
virCapsGuestPtr guest;
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 3da5fa4..f7edc02 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -42,7 +42,8 @@ static const char *domname_fc4 = fc4\n\n;
static const char *domstate_fc4 = running\n\n;
static int testFilterLine(char *buffer,
- const char
[libvirt] [PATCH v2 03/16] Use KR style for curly braces in src/util/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/util/vircgroup.c | 9 ++---
src/util/virconf.c | 5 +++--
src/util/virdbus.c | 8 +---
src/util/virerror.c | 3 ++-
src/util/vireventpoll.c | 29 +++--
src/util/virhook.c | 11 +++
src/util/virnetdevvportprofile.c | 5 +++--
src/util/virrandom.c | 5 +++--
src/util/virsocketaddr.c | 26 +-
src/util/virsysinfo.c| 15 ++-
src/util/virthread.c | 5 +++--
src/util/virutil.c | 20 +---
src/util/virutil.h | 12
src/util/viruuid.c | 5 +++--
14 files changed, 102 insertions(+), 56 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index c5925b1..84847b2 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -419,7 +419,8 @@ virCgroupCopyPlacement(virCgroupPtr group,
/*
* parent == / + path= = /
* parent == /libvirt.service + path == = /libvirt.service
- * parent == /libvirt.service + path == foo =
/libvirt.service/foo
+ * parent == /libvirt.service +
+ * path == foo = /libvirt.service/foo
*/
if (virAsprintf(group-controllers[i].placement,
%s%s%s,
@@ -519,8 +520,10 @@ virCgroupDetectPlacement(virCgroupPtr group,
/*
* selfpath == / + path= - /
- * selfpath == /libvirt.service + path == -
/libvirt.service
- * selfpath == /libvirt.service + path == foo -
/libvirt.service/foo
+ * selfpath == /libvirt.service +
+ * path == - /libvirt.service
+ * selfpath == /libvirt.service +
+ * path == foo -
/libvirt.service/foo
*/
if (typelen == len STREQLEN(typestr, tmp, len)
group-controllers[i].mountPoint != NULL
diff --git a/src/util/virconf.c b/src/util/virconf.c
index b965df7..233ad40 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1,7 +1,7 @@
/*
* virconf.c: parser for a subset of the Python encoded Xen configuration files
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -708,7 +708,8 @@ virConfParseStatement(virConfParserCtxtPtr ctxt)
*/
static virConfPtr
virConfParse(const char *filename, const char *content, int len,
- unsigned int flags) {
+ unsigned int flags)
+{
virConfParserCtxt ctxt;
ctxt.filename = filename;
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index e3236d8..9e29ca9 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1,7 +1,7 @@
/*
* virdbus.c: helper for using DBus
*
- * Copyright (C) 2012-2013 Red Hat, Inc.
+ * Copyright (C) 2012-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -217,7 +217,8 @@ static int virDBusTranslateWatchFlags(int dbus_flags)
}
-static void virDBusWatchFree(void *data) {
+static void virDBusWatchFree(void *data)
+{
struct virDBusWatch *info = data;
VIR_FREE(info);
}
@@ -296,7 +297,8 @@ static const char virDBusBasicTypes[] = {
DBUS_TYPE_SIGNATURE,
};
-static bool virDBusIsBasicType(char c) {
+static bool virDBusIsBasicType(char c)
+{
return !!memchr(virDBusBasicTypes, c,
ARRAY_CARDINALITY(virDBusBasicTypes));
}
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 1d7fa77..9db2452 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -43,7 +43,8 @@ virErrorFunc virErrorHandler = NULL; /* global error
handler */
void *virUserData = NULL;/* associated data */
virErrorLogPriorityFunc virErrorLogPriorityFilter = NULL;
-static virLogPriority virErrorLevelPriority(virErrorLevel level) {
+static virLogPriority virErrorLevelPriority(virErrorLevel level)
+{
switch (level) {
case VIR_ERR_NONE:
return VIR_LOG_INFO;
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index ea890de..d8a36e9 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -1,7 +1,7 @@
/*
* vireventpoll.c: Poll based event loop for monitoring file handles
*
- * Copyright (C) 2007, 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2007, 2010-2014 Red Hat, Inc.
* Copyright (C) 2007 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -106,7 +106,8 @@ static int nextTimer = 1;
int virEventPollAddHandle(int fd, int events,
virEventHandleCallback cb
[libvirt] [PATCH v2 02/16] Use KR style for curly braces in src/xen*/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/xen/xen_driver.c | 6 --
src/xen/xen_hypervisor.c | 5 +++--
src/xen/xm_internal.c | 10 +++---
src/xenapi/xenapi_utils.c | 5 +++--
src/xenxs/xen_xm.c| 35 +++
5 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 8ceb8b6..2199cb0 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -164,7 +164,8 @@ static virDomainDefPtr xenGetDomainDefForDom(virDomainPtr
dom)
* until reboot which might be false in future Xen implementations.
*/
static void
-xenNumaInit(virConnectPtr conn) {
+xenNumaInit(virConnectPtr conn)
+{
virNodeInfo nodeInfo;
xenUnifiedPrivatePtr priv;
int ret;
@@ -1916,7 +1917,8 @@ cleanup:
}
static int
-xenUnifiedDomainUndefine(virDomainPtr dom) {
+xenUnifiedDomainUndefine(virDomainPtr dom)
+{
return xenUnifiedDomainUndefineFlags(dom, 0);
}
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 5ccd5fa..e8eaeeb 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -1,7 +1,7 @@
/*
* xen_hypervisor.c: direct access to Xen hypervisor level
*
- * Copyright (C) 2005-2013 Red Hat, Inc.
+ * Copyright (C) 2005-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -2006,7 +2006,8 @@ xenHypervisorInit(struct xenHypervisorVersions
*override_versions)
}
-static int xenHypervisorOnceInit(void) {
+static int xenHypervisorOnceInit(void)
+{
return xenHypervisorInit(NULL);
}
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index fbdd89e..846b79c 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -1,7 +1,7 @@
/*
* xm_internal.c: helper routines for dealing with inactive domains
*
- * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -94,7 +94,8 @@ static int xenInotifyActive(virConnectPtr conn)
/* Release memory associated with a cached config object */
-static void xenXMConfigFree(void *payload, const void *key ATTRIBUTE_UNUSED) {
+static void xenXMConfigFree(void *payload, const void *key ATTRIBUTE_UNUSED)
+{
xenXMConfCachePtr entry = (xenXMConfCachePtr)payload;
virDomainDefFree(entry-def);
VIR_FREE(entry-filename);
@@ -1117,7 +1118,10 @@ struct xenXMListIteratorContext {
};
static void
-xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void
*data) {
+xenXMListIterator(void *payload ATTRIBUTE_UNUSED,
+ const void *name,
+ void *data)
+{
struct xenXMListIteratorContext *ctx = data;
virDomainDefPtr def = NULL;
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index 610e0f0..5a5025a 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -1,6 +1,6 @@
/*
* xenapi_utils.c: Xen API driver -- utils parts.
- * Copyright (C) 2011-2013 Red Hat, Inc.
+ * Copyright (C) 2011-2014 Red Hat, Inc.
* Copyright (C) 2009, 2010 Citrix Ltd.
*
* This library is free software; you can redistribute it and/or
@@ -181,7 +181,8 @@ createXenAPIBootOrderString(int nboot, int *bootDevs)
/* convert boot order string to libvirt boot order enum */
enum virDomainBootOrder
-map2LibvirtBootOrder(char c) {
+map2LibvirtBootOrder(char c)
+{
switch (c) {
case 'a':
return VIR_DOMAIN_BOOT_FLOPPY;
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index a70c5e3..fce074a 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1,7 +1,7 @@
/*
* xen_xm.c: Xen XM parsing functions
*
- * Copyright (C) 2006-2007, 2009-2010, 2012-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2010, 2012-2014 Red Hat, Inc.
* Copyright (C) 2011 Univention GmbH
* Copyright (C) 2006 Daniel P. Berrange
*
@@ -44,7 +44,8 @@
static int xenXMConfigGetBool(virConfPtr conf,
const char *name,
int *value,
- int def) {
+ int def)
+{
virConfValuePtr val;
*value = 0;
@@ -70,7 +71,8 @@ static int xenXMConfigGetBool(virConfPtr conf,
static int xenXMConfigGetULong(virConfPtr conf,
const char *name,
unsigned long *value,
- unsigned long def) {
+ unsigned long def)
+{
virConfValuePtr val;
*value = 0;
@@ -102,7 +104,8 @@ static int xenXMConfigGetULong(virConfPtr conf,
static int xenXMConfigGetULongLong(virConfPtr conf,
const char *name,
unsigned long long *value,
- unsigned long long def
[libvirt] [PATCH v2 04/16] Use KR style for curly braces in src/rpc/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/rpc/virnetserver.c | 8 +---
src/rpc/virnetserverclient.c | 5 +++--
src/rpc/virnettlscontext.c | 5 +++--
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index cea2b23..14fd102 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -1,7 +1,7 @@
/*
* virnetserver.c: generic network RPC server
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2012, 2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -850,7 +850,8 @@ static void
virNetServerSignalEvent(int watch,
int fd ATTRIBUTE_UNUSED,
int events ATTRIBUTE_UNUSED,
-void *opaque) {
+void *opaque)
+{
virNetServerPtr srv = opaque;
siginfo_t siginfo;
size_t i;
@@ -1021,7 +1022,8 @@ int virNetServerSetTLSContext(virNetServerPtr srv,
static void virNetServerAutoShutdownTimer(int timerid ATTRIBUTE_UNUSED,
- void *opaque) {
+ void *opaque)
+{
virNetServerPtr srv = opaque;
virObjectLock(srv);
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 495b0b3..1251b2d 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1,7 +1,7 @@
/*
* virnetserverclient.c: generic network RPC server client
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -141,7 +141,8 @@ static int
virNetServerClientSendMessageLocked(virNetServerClientPtr client,
* @client: a locked client object
*/
static int
-virNetServerClientCalculateHandleMode(virNetServerClientPtr client) {
+virNetServerClientCalculateHandleMode(virNetServerClientPtr client)
+{
int mode = 0;
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 7bf2a2e..4eaf469 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -1,7 +1,7 @@
/*
* virnettlscontext.c: TLS encryption/x509 handling
*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -125,7 +125,8 @@ virNetTLSContextCheckCertFile(const char *type, const char
*file, bool allowMiss
static void virNetTLSLog(int level ATTRIBUTE_UNUSED,
- const char *str ATTRIBUTE_UNUSED) {
+ const char *str ATTRIBUTE_UNUSED)
+{
VIR_DEBUG(%d %s, level, str);
}
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 11/16] Use KR style for curly braces in src/uml/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/uml/uml_conf.c | 5 ++--
src/uml/uml_driver.c | 78 ++--
2 files changed, 55 insertions(+), 28 deletions(-)
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index 3567b03..53a880f 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -1,7 +1,7 @@
/*
* uml_conf.c: UML driver configuration
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -52,7 +52,8 @@
VIR_LOG_INIT(uml.uml_conf);
-virCapsPtr umlCapsInit(void) {
+virCapsPtr umlCapsInit(void)
+{
virCapsPtr caps;
virCapsGuestPtr guest;
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 28e65f4..f5eb05f 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -209,7 +209,8 @@ umlAutostartDomain(virDomainObjPtr vm,
}
static void
-umlAutostartConfigs(struct uml_driver *driver) {
+umlAutostartConfigs(struct uml_driver *driver)
+{
/* XXX: Figure out a better way todo this. The domain
* startup code needs a connection handle in order
* to lookup the bridge associated with a virtual
@@ -622,7 +623,8 @@ static void umlNotifyLoadDomain(virDomainObjPtr vm, int
newVM, void *opaque)
* files and update its state and the networking
*/
static int
-umlStateReload(void) {
+umlStateReload(void)
+{
if (!uml_driver)
return 0;
@@ -660,7 +662,8 @@ umlShutdownOneVM(virDomainObjPtr dom, void *opaque)
* Shutdown the Uml daemon, it will stop all active domains and networks
*/
static int
-umlStateCleanup(void) {
+umlStateCleanup(void)
+{
if (!uml_driver)
return -1;
@@ -859,7 +862,8 @@ static int umlMonitorAddress(const struct uml_driver
*driver,
}
static int umlOpenMonitor(struct uml_driver *driver,
- virDomainObjPtr vm) {
+ virDomainObjPtr vm)
+{
struct sockaddr_un addr;
struct stat sb;
int retries = 0;
@@ -1007,7 +1011,8 @@ error:
}
-static void umlCleanupTapDevices(virDomainObjPtr vm) {
+static void umlCleanupTapDevices(virDomainObjPtr vm)
+{
size_t i;
for (i = 0; i vm-def-nnets; i++) {
@@ -1024,7 +1029,8 @@ static void umlCleanupTapDevices(virDomainObjPtr vm) {
static int umlStartVMDaemon(virConnectPtr conn,
struct uml_driver *driver,
virDomainObjPtr vm,
-bool autoDestroy) {
+bool autoDestroy)
+{
int ret = -1;
char *logfile;
int logfd = -1;
@@ -1245,7 +1251,8 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn,
return VIR_DRV_OPEN_SUCCESS;
}
-static int umlConnectClose(virConnectPtr conn) {
+static int umlConnectClose(virConnectPtr conn)
+{
struct uml_driver *driver = conn-privateData;
umlDriverLock(driver);
@@ -1343,7 +1350,8 @@ static int umlGetProcessInfo(unsigned long long *cpuTime,
pid_t pid)
static virDomainPtr umlDomainLookupByID(virConnectPtr conn,
- int id) {
+ int id)
+{
struct uml_driver *driver = (struct uml_driver *)conn-privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
@@ -1370,7 +1378,8 @@ cleanup:
}
static virDomainPtr umlDomainLookupByUUID(virConnectPtr conn,
-const unsigned char *uuid) {
+const unsigned char *uuid)
+{
struct uml_driver *driver = (struct uml_driver *)conn-privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
@@ -1397,7 +1406,8 @@ cleanup:
}
static virDomainPtr umlDomainLookupByName(virConnectPtr conn,
-const char *name) {
+const char *name)
+{
struct uml_driver *driver = (struct uml_driver *)conn-privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
@@ -1500,7 +1510,8 @@ cleanup:
return ret;
}
-static int umlConnectGetVersion(virConnectPtr conn, unsigned long *version) {
+static int umlConnectGetVersion(virConnectPtr conn, unsigned long *version)
+{
struct uml_driver *driver = conn-privateData;
struct utsname ut;
int ret = -1;
@@ -1538,7 +1549,8 @@ static char *umlConnectGetHostname(virConnectPtr conn)
}
-static int umlConnectListDomains(virConnectPtr conn, int *ids, int nids) {
+static int umlConnectListDomains(virConnectPtr conn, int *ids, int nids)
+{
struct uml_driver *driver = conn-privateData;
int n;
@@ -1552,7 +1564,8 @@ static int umlConnectListDomains(virConnectPtr conn, int
*ids, int nids) {
return n;
}
-static int umlConnectNumOfDomains(virConnectPtr conn) {
+static int umlConnectNumOfDomains(virConnectPtr conn)
+{
struct uml_driver *driver = conn-privateData
[libvirt] [PATCH v2 12/16] Use KR style for curly braces in src/lxc/lxc_driver.c
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/lxc/lxc_driver.c | 30 --
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 1ae04c5..3104bf9 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -384,7 +384,8 @@ cleanup:
return ret;
}
-static int lxcConnectListDomains(virConnectPtr conn, int *ids, int nids) {
+static int lxcConnectListDomains(virConnectPtr conn, int *ids, int nids)
+{
virLXCDriverPtr driver = conn-privateData;
int n;
@@ -397,7 +398,8 @@ static int lxcConnectListDomains(virConnectPtr conn, int
*ids, int nids) {
return n;
}
-static int lxcConnectNumOfDomains(virConnectPtr conn) {
+static int lxcConnectNumOfDomains(virConnectPtr conn)
+{
virLXCDriverPtr driver = conn-privateData;
int n;
@@ -411,7 +413,8 @@ static int lxcConnectNumOfDomains(virConnectPtr conn) {
}
static int lxcConnectListDefinedDomains(virConnectPtr conn,
-char **const names, int nnames) {
+char **const names, int nnames)
+{
virLXCDriverPtr driver = conn-privateData;
int n;
@@ -425,7 +428,8 @@ static int lxcConnectListDefinedDomains(virConnectPtr conn,
}
-static int lxcConnectNumOfDefinedDomains(virConnectPtr conn) {
+static int lxcConnectNumOfDefinedDomains(virConnectPtr conn)
+{
virLXCDriverPtr driver = conn-privateData;
int n;
@@ -677,7 +681,8 @@ cleanup:
return ret;
}
-static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) {
+static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax)
+{
virDomainObjPtr vm;
int ret = -1;
@@ -702,7 +707,8 @@ cleanup:
return ret;
}
-static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
+static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem)
+{
virDomainObjPtr vm;
int ret = -1;
virLXCDomainObjPrivatePtr priv;
@@ -1130,7 +1136,8 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn,
const char *xml,
unsigned int nfiles,
int *files,
-unsigned int flags) {
+unsigned int flags)
+{
virLXCDriverPtr driver = conn-privateData;
virDomainObjPtr vm = NULL;
virDomainDefPtr def = NULL;
@@ -1206,7 +1213,8 @@ cleanup:
static virDomainPtr
lxcDomainCreateXML(virConnectPtr conn,
const char *xml,
- unsigned int flags) {
+ unsigned int flags)
+{
return lxcDomainCreateXMLWithFiles(conn, xml, 0, NULL, flags);
}
@@ -1639,7 +1647,8 @@ static void lxcNotifyLoadDomain(virDomainObjPtr vm, int
newVM, void *opaque)
* files and perform autostart
*/
static int
-lxcStateReload(void) {
+lxcStateReload(void)
+{
virLXCDriverConfigPtr cfg = NULL;
virCapsPtr caps = NULL;
@@ -3102,7 +3111,8 @@ lxcDomainInterfaceStats(virDomainPtr dom,
#endif
static int lxcDomainGetAutostart(virDomainPtr dom,
- int *autostart) {
+ int *autostart)
+{
virDomainObjPtr vm;
int ret = -1;
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 16/16] Require KR styled curly braces around function bodies
Although not explicitly requested, we are using KR (or Kernel)
indentation for curly braces around functions in HACKING file and most
of the code. Using grep -P, this patch add the syntax-check rule for
it (while skipping all the false positives with foreach constructs).
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
Unfortunately, the regexp is meant to catch as much as possible and
thus one line functions must occupy two lines, e.g.:
static inline int getuid(void) { return 0; }
is not valid. This can be changed by appending $$ to the end of the
regexp which I didn't want to do since it might not catch something
else I haven't thought of.
Anyway, feel free to request such change and I'll push it changed
without any modifications to such one-liners.
cfg.mk | 7 +++
1 file changed, 7 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index 7a65d1e..559f719 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -898,6 +898,13 @@ sc_prohibit_virConnectOpen_in_virsh:
halt='Use vshConnect() in virsh instead of virConnectOpen*'\
$(_sc_search_regexp)
+sc_curly_braces_style:
+ @files=$$($(VC_LIST_EXCEPT) | grep '\.[ch]$$');\
+ $(GREP) -nHP \
+'^\s*(?!([a-zA-Z_]*for_?each[a-zA-Z_]*) ?\()([_a-zA-Z0-9]+( [_a-zA-Z0-9]+)*
?\()?(\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9\[\]]+)+|void)\) ?\{' \
+ $$files { echo '$(ME): Non-KR style used for curly'\
+ 'braces around function body, see' \
+ 'HACKING' 12; exit 1; } || :
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 06/16] Use KR style for curly braces in src/qemu/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_agent.c| 5 ++-
src/qemu/qemu_command.c | 6 ++-
src/qemu/qemu_driver.c | 94 +---
src/qemu/qemu_migration.c| 3 +-
src/qemu/qemu_monitor.c | 3 +-
src/qemu/qemu_monitor_json.c | 3 +-
src/qemu/qemu_monitor_text.c | 20 +++---
7 files changed, 90 insertions(+), 44 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 83f077f..9f02e52 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1,7 +1,7 @@
/*
* qemu_agent.c: interaction with QEMU guest agent
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -574,7 +574,8 @@ static void qemuAgentUpdateWatch(qemuAgentPtr mon)
static void
-qemuAgentIO(int watch, int fd, int events, void *opaque) {
+qemuAgentIO(int watch, int fd, int events, void *opaque)
+{
qemuAgentPtr mon = opaque;
bool error = false;
bool eof = false;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5ca3f74..2311d89 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1871,7 +1871,8 @@ cleanup:
}
static bool
-qemuDomainSupportsPCI(virDomainDefPtr def) {
+qemuDomainSupportsPCI(virDomainDefPtr def)
+{
if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch !=
VIR_ARCH_AARCH64))
return true;
@@ -11197,7 +11198,8 @@ error:
static void
-qemuParseCommandLineBootDevs(virDomainDefPtr def, const char *str) {
+qemuParseCommandLineBootDevs(virDomainDefPtr def, const char *str)
+{
int n, b = 0;
for (n = 0; str[n] b VIR_DOMAIN_BOOT_LAST; n++) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2707bec..20239f1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -172,9 +172,11 @@ virQEMUDriverPtr qemu_driver = NULL;
static void
-qemuVMDriverLock(void) {}
+qemuVMDriverLock(void)
+{}
static void
-qemuVMDriverUnlock(void) {}
+qemuVMDriverUnlock(void)
+{}
static int
qemuVMFilterRebuild(virDomainObjListIterator iter, void *data)
@@ -879,7 +881,8 @@ static void qemuNotifyLoadDomain(virDomainObjPtr vm, int
newVM, void *opaque)
* files and update its state and the networking
*/
static int
-qemuStateReload(void) {
+qemuStateReload(void)
+{
virQEMUDriverConfigPtr cfg = NULL;
virCapsPtr caps = NULL;
@@ -910,7 +913,8 @@ cleanup:
*
*/
static int
-qemuStateStop(void) {
+qemuStateStop(void)
+{
int ret = -1;
virConnectPtr conn;
int numDomains = 0;
@@ -967,7 +971,8 @@ qemuStateStop(void) {
* Shutdown the QEmu daemon, it will stop all active domains and networks
*/
static int
-qemuStateCleanup(void) {
+qemuStateCleanup(void)
+{
if (!qemu_driver)
return -1;
@@ -1145,7 +1150,8 @@ static int qemuConnectIsAlive(virConnectPtr conn
ATTRIBUTE_UNUSED)
static int
-kvmGetMaxVCPUs(void) {
+kvmGetMaxVCPUs(void)
+{
int fd;
int ret;
@@ -1201,7 +1207,9 @@ qemuConnectGetSysinfo(virConnectPtr conn, unsigned int
flags)
return virBufferContentAndReset(buf);
}
-static int qemuConnectGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, const
char *type) {
+static int
+qemuConnectGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, const char *type)
+{
if (virConnectGetMaxVcpusEnsureACL(conn) 0)
return -1;
@@ -1321,7 +1329,8 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int
*lastCpu, long *vm_rss,
static virDomainPtr qemuDomainLookupByID(virConnectPtr conn,
- int id) {
+ int id)
+{
virQEMUDriverPtr driver = conn-privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
@@ -1347,7 +1356,8 @@ cleanup:
}
static virDomainPtr qemuDomainLookupByUUID(virConnectPtr conn,
- const unsigned char *uuid) {
+ const unsigned char *uuid)
+{
virQEMUDriverPtr driver = conn-privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
@@ -1375,7 +1385,8 @@ cleanup:
}
static virDomainPtr qemuDomainLookupByName(virConnectPtr conn,
- const char *name) {
+ const char *name)
+{
virQEMUDriverPtr driver = conn-privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
@@ -1458,7 +1469,8 @@ cleanup:
return ret;
}
-static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version) {
+static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version)
+{
virQEMUDriverPtr driver = conn-privateData;
int ret = -1;
unsigned int qemuVersion = 0;
@@ -1493,7 +1505,8 @@ static char *qemuConnectGetHostname(virConnectPtr conn)
}
-static int qemuConnectListDomains(virConnectPtr conn, int *ids, int
[libvirt] [PATCH v2 08/16] Use KR style for curly braces in src/openvz/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/openvz/openvz_conf.c | 15 ++-
src/openvz/openvz_driver.c | 45 ++---
2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 11f048b..20c9a6f 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -205,7 +205,8 @@ no_memory:
int
openvzReadNetworkConf(virDomainDefPtr def,
- int veid) {
+ int veid)
+{
int ret;
virDomainNetDefPtr net = NULL;
char *temp = NULL;
@@ -378,7 +379,8 @@ openvz_replace(const char* str,
static int
openvzReadFSConf(virDomainDefPtr def,
- int veid) {
+ int veid)
+{
int ret;
virDomainFSDefPtr fs = NULL;
char *veid_str = NULL;
@@ -545,7 +547,8 @@ openvzFreeDriver(struct openvz_driver *driver)
-int openvzLoadDomains(struct openvz_driver *driver) {
+int openvzLoadDomains(struct openvz_driver *driver)
+{
int veid, ret;
char *status;
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -1063,7 +1066,8 @@ cleanup:
}
static int
-openvzSetUUID(int vpsid){
+openvzSetUUID(int vpsid)
+{
unsigned char uuid[VIR_UUID_BUFLEN];
if (virUUIDGenerate(uuid)) {
@@ -1128,7 +1132,8 @@ static int openvzAssignUUIDs(void)
*
*/
-int openvzGetVEID(const char *name) {
+int openvzGetVEID(const char *name)
+{
virCommandPtr cmd;
char *outbuf;
char *temp;
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 2a28ed2..526ddbd 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -299,7 +299,8 @@ error:
static virDomainPtr openvzDomainLookupByID(virConnectPtr conn,
- int id) {
+ int id)
+{
struct openvz_driver *driver = conn-privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
@@ -323,7 +324,8 @@ cleanup:
return dom;
}
-static int openvzConnectGetVersion(virConnectPtr conn, unsigned long *version)
{
+static int openvzConnectGetVersion(virConnectPtr conn, unsigned long *version)
+{
struct openvz_driver *driver = conn-privateData;
openvzDriverLock(driver);
*version = driver-version;
@@ -363,7 +365,8 @@ cleanup:
static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn,
- const unsigned char *uuid) {
+ const unsigned char *uuid)
+{
struct openvz_driver *driver = conn-privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
@@ -388,7 +391,8 @@ cleanup:
}
static virDomainPtr openvzDomainLookupByName(virConnectPtr conn,
- const char *name) {
+ const char *name)
+{
struct openvz_driver *driver = conn-privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
@@ -413,7 +417,8 @@ cleanup:
}
static int openvzDomainGetInfo(virDomainPtr dom,
- virDomainInfoPtr info) {
+ virDomainInfoPtr info)
+{
struct openvz_driver *driver = dom-conn-privateData;
virDomainObjPtr vm;
int state;
@@ -579,7 +584,8 @@ static void openvzSetProgramSentinal(const char **prog,
const char *key)
}
}
-static int openvzDomainSuspend(virDomainPtr dom) {
+static int openvzDomainSuspend(virDomainPtr dom)
+{
struct openvz_driver *driver = dom-conn-privateData;
virDomainObjPtr vm;
const char *prog[] = {VZCTL, --quiet, chkpnt, PROGRAM_SENTINEL,
--suspend, NULL};
@@ -617,7 +623,8 @@ cleanup:
return ret;
}
-static int openvzDomainResume(virDomainPtr dom) {
+static int openvzDomainResume(virDomainPtr dom)
+{
struct openvz_driver *driver = dom-conn-privateData;
virDomainObjPtr vm;
const char *prog[] = {VZCTL, --quiet, chkpnt, PROGRAM_SENTINEL,
--resume, NULL};
@@ -657,7 +664,8 @@ cleanup:
static int
openvzDomainShutdownFlags(virDomainPtr dom,
- unsigned int flags) {
+ unsigned int flags)
+{
struct openvz_driver *driver = dom-conn-privateData;
virDomainObjPtr vm;
const char *prog[] = {VZCTL, --quiet, stop, PROGRAM_SENTINEL, NULL};
@@ -1476,7 +1484,8 @@ cleanup:
return VIR_DRV_OPEN_ERROR;
};
-static int openvzConnectClose(virConnectPtr conn) {
+static int openvzConnectClose(virConnectPtr conn)
+{
struct openvz_driver *driver = conn-privateData;
openvzFreeDriver(driver);
@@ -1489,12 +1498,14 @@ static const char *openvzConnectGetType(virConnectPtr
conn ATTRIBUTE_UNUSED) {
return OpenVZ;
}
-static int openvzConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) {
+static int openvzConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED)
+{
/* Encryption is not relevant / applicable to way we talk to openvz */
return 0
[libvirt] [PATCH v2 05/16] Use KR style for curly braces in src/conf/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c | 9 +---
src/conf/domain_nwfilter.c | 13 +++
src/conf/interface_conf.c | 54 ++
src/conf/nwfilter_conf.c | 30 +-
src/conf/nwfilter_params.c | 3 ++-
5 files changed, 73 insertions(+), 36 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 89aa52c..081ec8d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6193,7 +6193,8 @@ virDomainNetGenerateMAC(virDomainXMLOptionPtr xmlopt,
static virDomainFSDefPtr
virDomainFSDefParseXML(xmlNodePtr node,
xmlXPathContextPtr ctxt,
- unsigned int flags) {
+ unsigned int flags)
+{
virDomainFSDefPtr def;
xmlNodePtr cur, save_node = ctxt-node;
char *type = NULL;
@@ -7013,7 +7014,8 @@ error:
}
static int
-virDomainChrDefaultTargetType(int devtype) {
+virDomainChrDefaultTargetType(int devtype)
+{
switch ((enum virDomainChrDeviceType) devtype) {
case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL:
virReportError(VIR_ERR_XML_ERROR,
@@ -7419,7 +7421,8 @@ error:
* default port.
*/
virDomainChrDefPtr
-virDomainChrDefNew(void) {
+virDomainChrDefNew(void)
+{
virDomainChrDefPtr def = NULL;
if (VIR_ALLOC(def) 0)
diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c
index 688a200..6330f67 100644
--- a/src/conf/domain_nwfilter.c
+++ b/src/conf/domain_nwfilter.c
@@ -1,6 +1,7 @@
/*
* domain_nwfilter.c:
*
+ * Copyright (C) 2014 Red Hat, Inc.
* Copyright (C) 2010 IBM Corporation
*
* This library is free software; you can redistribute it and/or
@@ -31,14 +32,16 @@
static virDomainConfNWFilterDriverPtr nwfilterDriver;
void
-virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver) {
+virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver)
+{
nwfilterDriver = driver;
}
int
virDomainConfNWFilterInstantiate(virConnectPtr conn,
const unsigned char *vmuuid,
- virDomainNetDefPtr net) {
+ virDomainNetDefPtr net)
+{
if (nwfilterDriver != NULL)
return nwfilterDriver-instantiateFilter(conn, vmuuid, net);
/* driver module not available -- don't indicate failure */
@@ -46,13 +49,15 @@ virDomainConfNWFilterInstantiate(virConnectPtr conn,
}
void
-virDomainConfNWFilterTeardown(virDomainNetDefPtr net) {
+virDomainConfNWFilterTeardown(virDomainNetDefPtr net)
+{
if (nwfilterDriver != NULL)
nwfilterDriver-teardownFilter(net);
}
void
-virDomainConfVMNWFilterTeardown(virDomainObjPtr vm) {
+virDomainConfVMNWFilterTeardown(virDomainObjPtr vm)
+{
size_t i;
if (nwfilterDriver != NULL) {
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 9f065bf..83cc0a9 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -44,7 +44,8 @@ static int
virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def);
static
-void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
+void virInterfaceIpDefFree(virInterfaceIpDefPtr def)
+{
if (def == NULL)
return;
VIR_FREE(def-address);
@@ -52,7 +53,8 @@ void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
}
static
-void virInterfaceProtocolDefFree(virInterfaceProtocolDefPtr def) {
+void virInterfaceProtocolDefFree(virInterfaceProtocolDefPtr def)
+{
size_t i;
if (def == NULL)
@@ -112,7 +114,8 @@ void virInterfaceDefFree(virInterfaceDefPtr def)
static int
virInterfaceDefParseName(virInterfaceDefPtr def,
- xmlXPathContextPtr ctxt) {
+ xmlXPathContextPtr ctxt)
+{
char *tmp;
tmp = virXPathString(string(./@name), ctxt);
@@ -127,7 +130,8 @@ virInterfaceDefParseName(virInterfaceDefPtr def,
static int
virInterfaceDefParseMtu(virInterfaceDefPtr def,
-xmlXPathContextPtr ctxt) {
+xmlXPathContextPtr ctxt)
+{
unsigned long mtu;
int ret;
@@ -144,7 +148,8 @@ virInterfaceDefParseMtu(virInterfaceDefPtr def,
static int
virInterfaceDefParseStartMode(virInterfaceDefPtr def,
- xmlXPathContextPtr ctxt) {
+ xmlXPathContextPtr ctxt)
+{
char *tmp;
tmp = virXPathString(string(./start/@mode), ctxt);
@@ -167,7 +172,8 @@ virInterfaceDefParseStartMode(virInterfaceDefPtr def,
}
static int
-virInterfaceDefParseBondMode(xmlXPathContextPtr ctxt) {
+virInterfaceDefParseBondMode(xmlXPathContextPtr ctxt)
+{
char *tmp;
int ret = 0;
@@ -198,7 +204,8 @@ virInterfaceDefParseBondMode(xmlXPathContextPtr ctxt) {
}
static int
-virInterfaceDefParseBondMiiCarrier(xmlXPathContextPtr ctxt) {
+virInterfaceDefParseBondMiiCarrier(xmlXPathContextPtr ctxt)
+{
char *tmp;
int ret = 0;
@@ -219,7 +226,8
[libvirt] [PATCH v2 14/16] Use KR style for curly braces in src/vbox/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/vbox/vbox_driver.c | 5 +-
src/vbox/vbox_tmpl.c | 188 +
2 files changed, 132 insertions(+), 61 deletions(-)
diff --git a/src/vbox/vbox_driver.c b/src/vbox/vbox_driver.c
index e26b10a..7d004b2 100644
--- a/src/vbox/vbox_driver.c
+++ b/src/vbox/vbox_driver.c
@@ -3,7 +3,7 @@
*/
/*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
* Copyright (C) 2008-2009 Sun Microsystems, Inc.
*
* This file is part of a free software library; you can redistribute
@@ -79,7 +79,8 @@ static virDriver vboxDriverDummy;
#define VIR_FROM_THIS VIR_FROM_VBOX
-int vboxRegister(void) {
+int vboxRegister(void)
+{
virDriverPtrdriver;
virNetworkDriverPtr networkDriver;
virStorageDriverPtr storageDriver;
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 2aeddd0..56b3ac6 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -284,17 +284,20 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr
conn, const char *xml);
static int vboxDomainCreate(virDomainPtr dom);
static int vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags);
-static void vboxDriverLock(vboxGlobalData *data) {
+static void vboxDriverLock(vboxGlobalData *data)
+{
virMutexLock(data-lock);
}
-static void vboxDriverUnlock(vboxGlobalData *data) {
+static void vboxDriverUnlock(vboxGlobalData *data)
+{
virMutexUnlock(data-lock);
}
#if VBOX_API_VERSION == 2002000
-static void nsIDtoChar(unsigned char *uuid, const nsID *iid) {
+static void nsIDtoChar(unsigned char *uuid, const nsID *iid)
+{
char uuidstrsrc[VIR_UUID_STRING_BUFLEN];
char uuidstrdst[VIR_UUID_STRING_BUFLEN];
unsigned char uuidinterim[VIR_UUID_BUFLEN];
@@ -334,7 +337,8 @@ static void nsIDtoChar(unsigned char *uuid, const nsID
*iid) {
ignore_value(virUUIDParse(uuidstrdst, uuid));
}
-static void nsIDFromChar(nsID *iid, const unsigned char *uuid) {
+static void nsIDFromChar(nsID *iid, const unsigned char *uuid)
+{
char uuidstrsrc[VIR_UUID_STRING_BUFLEN];
char uuidstrdst[VIR_UUID_STRING_BUFLEN];
unsigned char uuidinterim[VIR_UUID_BUFLEN];
@@ -621,7 +625,8 @@ static char *vboxGenerateMediumName(PRUint32 storageBus,
PRInt32 devicePort,
PRInt32 deviceSlot,
PRUint32 *aMaxPortPerInst,
-PRUint32 *aMaxSlotPerPort) {
+PRUint32 *aMaxSlotPerPort)
+{
const char *prefix = NULL;
char *name = NULL;
int total = 0;
@@ -734,7 +739,8 @@ static bool vboxGetDeviceDetails(const char *deviceName,
static bool vboxGetMaxPortSlotValues(IVirtualBox *vbox,
PRUint32 *maxPortPerInst,
- PRUint32 *maxSlotPerPort) {
+ PRUint32 *maxSlotPerPort)
+{
ISystemProperties *sysProps = NULL;
if (!vbox)
@@ -779,7 +785,8 @@ static bool vboxGetMaxPortSlotValues(IVirtualBox *vbox,
/**
* Converts Utf-16 string to int
*/
-static int PRUnicharToInt(PRUnichar *strUtf16) {
+static int PRUnicharToInt(PRUnichar *strUtf16)
+{
char *strUtf8 = NULL;
int ret = 0;
@@ -957,7 +964,8 @@ cleanup:
return -1;
}
-static int vboxExtractVersion(vboxGlobalData *data) {
+static int vboxExtractVersion(vboxGlobalData *data)
+{
int ret = -1;
PRUnichar *versionUtf16 = NULL;
nsresult rc;
@@ -985,7 +993,8 @@ static int vboxExtractVersion(vboxGlobalData *data) {
return ret;
}
-static void vboxUninitialize(vboxGlobalData *data) {
+static void vboxUninitialize(vboxGlobalData *data)
+{
if (!data)
return;
@@ -1078,7 +1087,8 @@ static virDrvOpenStatus vboxConnectOpen(virConnectPtr
conn,
return VIR_DRV_OPEN_SUCCESS;
}
-static int vboxConnectClose(virConnectPtr conn) {
+static int vboxConnectClose(virConnectPtr conn)
+{
vboxGlobalData *data = conn-privateData;
VIR_DEBUG(%s: in vboxClose, conn-driver-name);
@@ -1088,7 +1098,8 @@ static int vboxConnectClose(virConnectPtr conn) {
return 0;
}
-static int vboxConnectGetVersion(virConnectPtr conn, unsigned long *version) {
+static int vboxConnectGetVersion(virConnectPtr conn, unsigned long *version)
+{
vboxGlobalData *data = conn-privateData;
VIR_DEBUG(%s: in vboxGetVersion, conn-driver-name);
@@ -1106,12 +1117,14 @@ static char *vboxConnectGetHostname(virConnectPtr conn
ATTRIBUTE_UNUSED)
}
-static int vboxConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) {
+static int vboxConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED)
+{
/* Driver is using local, non-network based transport */
return 1;
}
-static int vboxConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) {
+static int vboxConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED
[libvirt] [PATCH v2 09/16] Use KR style for curly braces in src/nwfilter/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/nwfilter/nwfilter_driver.c| 32 +++-
src/nwfilter/nwfilter_ebiptables_driver.c | 3 ++-
src/nwfilter/nwfilter_learnipaddr.c | 41 ---
3 files changed, 50 insertions(+), 26 deletions(-)
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 0876901..4fab1f2 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -2,7 +2,7 @@
* nwfilter_driver.c: core driver for network filter APIs
*(based on storage_driver.c)
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2011, 2014 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
* Copyright (C) 2010 IBM Corporation
* Copyright (C) 2010 Stefan Berger
@@ -325,7 +325,8 @@ virNWFilterDriverIsWatchingFirewallD(void)
* Shutdown the nwfilter driver, it will stop all active nwfilters
*/
static int
-nwfilterStateCleanup(void) {
+nwfilterStateCleanup(void)
+{
if (!driverState)
return -1;
@@ -356,7 +357,8 @@ nwfilterStateCleanup(void) {
static virNWFilterPtr
nwfilterLookupByUUID(virConnectPtr conn,
- const unsigned char *uuid) {
+ const unsigned char *uuid)
+{
virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData;
virNWFilterObjPtr nwfilter;
virNWFilterPtr ret = NULL;
@@ -385,7 +387,8 @@ cleanup:
static virNWFilterPtr
nwfilterLookupByName(virConnectPtr conn,
- const char *name) {
+ const char *name)
+{
virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData;
virNWFilterObjPtr nwfilter;
virNWFilterPtr ret = NULL;
@@ -428,14 +431,16 @@ nwfilterOpen(virConnectPtr conn,
static int
-nwfilterClose(virConnectPtr conn) {
+nwfilterClose(virConnectPtr conn)
+{
conn-nwfilterPrivateData = NULL;
return 0;
}
static int
-nwfilterConnectNumOfNWFilters(virConnectPtr conn) {
+nwfilterConnectNumOfNWFilters(virConnectPtr conn)
+{
virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData;
size_t i;
int n;
@@ -459,7 +464,8 @@ nwfilterConnectNumOfNWFilters(virConnectPtr conn) {
static int
nwfilterConnectListNWFilters(virConnectPtr conn,
char **const names,
- int nnames) {
+ int nnames)
+{
virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData;
int got = 0;
size_t i;
@@ -495,7 +501,8 @@ nwfilterConnectListNWFilters(virConnectPtr conn,
static int
nwfilterConnectListAllNWFilters(virConnectPtr conn,
virNWFilterPtr **filters,
-unsigned int flags) {
+unsigned int flags)
+{
virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData;
virNWFilterPtr *tmp_filters = NULL;
int nfilters = 0;
@@ -594,7 +601,8 @@ cleanup:
static int
-nwfilterUndefine(virNWFilterPtr obj) {
+nwfilterUndefine(virNWFilterPtr obj)
+{
virNWFilterDriverStatePtr driver = obj-conn-nwfilterPrivateData;
virNWFilterObjPtr nwfilter;
int ret = -1;
@@ -682,7 +690,8 @@ nwfilterInstantiateFilter(virConnectPtr conn,
static void
-nwfilterTeardownFilter(virDomainNetDefPtr net) {
+nwfilterTeardownFilter(virDomainNetDefPtr net)
+{
if ((net-ifname) (net-filter))
virNWFilterTeardownFilter(net);
}
@@ -717,7 +726,8 @@ static virDomainConfNWFilterDriver domainNWFilterDriver = {
};
-int nwfilterRegister(void) {
+int nwfilterRegister(void)
+{
if (virRegisterNWFilterDriver(nwfilterDriver) 0)
return -1;
if (virRegisterStateDriver(stateDriver) 0)
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c
b/src/nwfilter/nwfilter_ebiptables_driver.c
index 6909a9a..e535356 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -3198,7 +3198,8 @@ ebiptablesInstCommand(virBufferPtr buf,
* In case of this driver we need the ebtables tool available.
*/
static int
-ebiptablesCanApplyBasicRules(void) {
+ebiptablesCanApplyBasicRules(void)
+{
return ebtables_cmd_path != NULL;
}
diff --git a/src/nwfilter/nwfilter_learnipaddr.c
b/src/nwfilter/nwfilter_learnipaddr.c
index e156a93..e01d4df 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -2,7 +2,7 @@
* nwfilter_learnipaddr.c: support for learning IP address used by a VM
* on an interface
*
- * Copyright (C) 2011, 2013 Red Hat, Inc.
+ * Copyright (C) 2011, 2013, 2014 Red Hat, Inc.
* Copyright (C) 2010 IBM Corp.
* Copyright (C) 2010 Stefan Berger
*
@@ -138,7 +138,8 @@ static bool threadsTerminate = false;
int
-virNWFilterLockIface(const char *ifname) {
+virNWFilterLockIface(const char *ifname)
+{
virNWFilterIfaceLockPtr ifaceLock;
virMutexLock(ifaceMapLock);
@@ -188,13 +189,15
[libvirt] [PATCH v2 10/16] Use KR style for curly braces in src/test/test_driver.c
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/test/test_driver.c | 135 -
1 file changed, 90 insertions(+), 45 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 5716449..d88d3fc 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -319,7 +319,8 @@ testBuildXMLConfig(void)
static virCapsPtr
-testBuildCapabilities(virConnectPtr conn) {
+testBuildCapabilities(virConnectPtr conn)
+{
testConnPtr privconn = conn-privateData;
virCapsPtr caps;
virCapsGuestPtr guest;
@@ -504,7 +505,8 @@ testDomObjFromDomain(virDomainPtr domain)
}
static char *
-testDomainGenerateIfname(virDomainDefPtr domdef) {
+testDomainGenerateIfname(virDomainDefPtr domdef)
+{
int maxif = 1024;
int ifctr;
size_t i;
@@ -851,7 +853,8 @@ error:
static char *testBuildFilename(const char *relativeTo,
- const char *filename) {
+ const char *filename)
+{
char *offset;
int baseLen;
char *ret;
@@ -2495,7 +2498,8 @@ static char *testDomainGetOSType(virDomainPtr dom
ATTRIBUTE_UNUSED) {
return ret;
}
-static unsigned long long testDomainGetMaxMemory(virDomainPtr domain) {
+static unsigned long long testDomainGetMaxMemory(virDomainPtr domain)
+{
testConnPtr privconn = domain-conn-privateData;
virDomainObjPtr privdom;
unsigned long long ret = 0;
@@ -2898,7 +2902,8 @@ cleanup:
return ret;
}
-static int testConnectNumOfDefinedDomains(virConnectPtr conn) {
+static int testConnectNumOfDefinedDomains(virConnectPtr conn)
+{
testConnPtr privconn = conn-privateData;
int count;
@@ -2911,7 +2916,8 @@ static int testConnectNumOfDefinedDomains(virConnectPtr
conn) {
static int testConnectListDefinedDomains(virConnectPtr conn,
char **const names,
- int maxnames) {
+ int maxnames)
+{
testConnPtr privconn = conn-privateData;
int n;
@@ -2926,7 +2932,8 @@ static int testConnectListDefinedDomains(virConnectPtr
conn,
}
static virDomainPtr testDomainDefineXML(virConnectPtr conn,
-const char *xml) {
+const char *xml)
+{
testConnPtr privconn = conn-privateData;
virDomainPtr ret = NULL;
virDomainDefPtr def;
@@ -3040,7 +3047,8 @@ cleanup:
static int testNodeGetCellsFreeMemory(virConnectPtr conn,
unsigned long long *freemems,
- int startCell, int maxCells) {
+ int startCell, int maxCells)
+{
testConnPtr privconn = conn-privateData;
int cell;
size_t i;
@@ -3066,7 +3074,8 @@ cleanup:
}
-static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) {
+static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags)
+{
testConnPtr privconn = domain-conn-privateData;
virDomainObjPtr privdom;
virObjectEventPtr event = NULL;
@@ -3108,7 +3117,8 @@ cleanup:
return ret;
}
-static int testDomainCreate(virDomainPtr domain) {
+static int testDomainCreate(virDomainPtr domain)
+{
return testDomainCreateWithFlags(domain, 0);
}
@@ -3475,7 +3485,8 @@ static virDrvOpenStatus testNetworkOpen(virConnectPtr
conn,
return VIR_DRV_OPEN_SUCCESS;
}
-static int testNetworkClose(virConnectPtr conn) {
+static int testNetworkClose(virConnectPtr conn)
+{
conn-networkPrivateData = NULL;
return 0;
}
@@ -3530,7 +3541,8 @@ cleanup:
}
-static int testConnectNumOfNetworks(virConnectPtr conn) {
+static int testConnectNumOfNetworks(virConnectPtr conn)
+{
testConnPtr privconn = conn-privateData;
int numActive = 0;
size_t i;
@@ -3574,7 +3586,8 @@ error:
return -1;
}
-static int testConnectNumOfDefinedNetworks(virConnectPtr conn) {
+static int testConnectNumOfDefinedNetworks(virConnectPtr conn)
+{
testConnPtr privconn = conn-privateData;
int numInactive = 0;
size_t i;
@@ -3678,7 +3691,8 @@ cleanup:
}
-static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const char *xml)
{
+static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const char *xml)
+{
testConnPtr privconn = conn-privateData;
virNetworkDefPtr def;
virNetworkObjPtr net = NULL;
@@ -3744,7 +3758,8 @@ cleanup:
return ret;
}
-static int testNetworkUndefine(virNetworkPtr network) {
+static int testNetworkUndefine(virNetworkPtr network)
+{
testConnPtr privconn = network-conn-privateData;
virNetworkObjPtr privnet;
int ret = -1;
@@ -3831,7 +3846,8 @@ cleanup:
return ret;
}
-static int testNetworkCreate(virNetworkPtr network) {
+static int testNetworkCreate(virNetworkPtr network)
+{
testConnPtr privconn = network-conn-privateData;
virNetworkObjPtr privnet;
int
[libvirt] [PATCH v2 07/16] Use KR style for curly braces in src/storage/
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/storage/storage_backend_fs.c | 12 ---
src/storage/storage_driver.c | 78 ++--
2 files changed, 60 insertions(+), 30 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index edb7cd0..722193b 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -330,7 +330,8 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr
conn ATTRIBUTE_UNUSE
* Return 0 if not mounted, 1 if mounted, -1 on error
*/
static int
-virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) {
+virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool)
+{
FILE *mtab;
struct mntent ent;
char buf[1024];
@@ -363,7 +364,8 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr
pool) {
* Returns 0 if successfully mounted, -1 on error
*/
static int
-virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) {
+virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
+{
char *src = NULL;
/* 'mount -t auto' doesn't seem to auto determine nfs (or cifs),
* while plain 'mount' does. We have to craft separate argvs to
@@ -467,7 +469,8 @@ cleanup:
* Returns 0 if successfully unmounted, -1 on error
*/
static int
-virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) {
+virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool)
+{
virCommandPtr cmd = NULL;
int ret = -1;
int rc;
@@ -562,7 +565,8 @@ virStorageBackendFileSystemStart(virConnectPtr conn
ATTRIBUTE_UNUSED,
#if WITH_BLKID
static virStoragePoolProbeResult
virStorageBackendFileSystemProbe(const char *device,
- const char *format) {
+ const char *format)
+{
virStoragePoolProbeResult ret = FILESYSTEM_PROBE_ERROR;
blkid_probe probe = NULL;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 6a2840c..3637aa5 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -68,7 +68,8 @@ static void storageDriverUnlock(virStorageDriverStatePtr
driver)
}
static void
-storageDriverAutostart(virStorageDriverStatePtr driver) {
+storageDriverAutostart(virStorageDriverStatePtr driver)
+{
size_t i;
virConnectPtr conn = NULL;
@@ -217,7 +218,8 @@ storageStateAutoStart(void)
* files and update its state
*/
static int
-storageStateReload(void) {
+storageStateReload(void)
+{
if (!driverState)
return -1;
@@ -238,7 +240,8 @@ storageStateReload(void) {
* Shutdown the storage driver, it will stop all active storage pools
*/
static int
-storageStateCleanup(void) {
+storageStateCleanup(void)
+{
if (!driverState)
return -1;
@@ -260,7 +263,8 @@ storageStateCleanup(void) {
static virStoragePoolPtr
storagePoolLookupByUUID(virConnectPtr conn,
-const unsigned char *uuid) {
+const unsigned char *uuid)
+{
virStorageDriverStatePtr driver = conn-storagePrivateData;
virStoragePoolObjPtr pool;
virStoragePoolPtr ret = NULL;
@@ -289,7 +293,8 @@ cleanup:
static virStoragePoolPtr
storagePoolLookupByName(virConnectPtr conn,
-const char *name) {
+const char *name)
+{
virStorageDriverStatePtr driver = conn-storagePrivateData;
virStoragePoolObjPtr pool;
virStoragePoolPtr ret = NULL;
@@ -317,7 +322,8 @@ cleanup:
}
static virStoragePoolPtr
-storagePoolLookupByVolume(virStorageVolPtr vol) {
+storagePoolLookupByVolume(virStorageVolPtr vol)
+{
virStorageDriverStatePtr driver = vol-conn-storagePrivateData;
virStoragePoolObjPtr pool;
virStoragePoolPtr ret = NULL;
@@ -359,13 +365,15 @@ storageOpen(virConnectPtr conn,
}
static int
-storageClose(virConnectPtr conn) {
+storageClose(virConnectPtr conn)
+{
conn-storagePrivateData = NULL;
return 0;
}
static int
-storageConnectNumOfStoragePools(virConnectPtr conn) {
+storageConnectNumOfStoragePools(virConnectPtr conn)
+{
virStorageDriverStatePtr driver = conn-storagePrivateData;
size_t i;
int nactive = 0;
@@ -390,7 +398,8 @@ storageConnectNumOfStoragePools(virConnectPtr conn) {
static int
storageConnectListStoragePools(virConnectPtr conn,
char **const names,
- int nnames) {
+ int nnames)
+{
virStorageDriverStatePtr driver = conn-storagePrivateData;
int got = 0;
size_t i;
@@ -424,7 +433,8 @@ storageConnectListStoragePools(virConnectPtr conn,
}
static int
-storageConnectNumOfDefinedStoragePools(virConnectPtr conn) {
+storageConnectNumOfDefinedStoragePools(virConnectPtr conn)
+{
virStorageDriverStatePtr driver = conn-storagePrivateData;
size_t i;
int nactive = 0;
@@ -449,7 +459,8 @@ storageConnectNumOfDefinedStoragePools(virConnectPtr conn) {
static
Re: [libvirt] [PATCH] nwfilter: Fix the test for the result of atomic dec and test
On Tue, Mar 18, 2014 at 09:45:14PM -0400, Stefan Berger wrote:
From: Stefan Berger stef...@linux.vnet.ibm.com
https://bugzilla.redhat.com/show_bug.cgi?id=1071181
Commit 49b59a15 fixed one problem but masks another one related to pointer
freeing.
Use virAtomicIntGet() to test for 0 rather than trying to test for 'true'
after virAtomicIntDecAndTest().
Avoid putting of the virNWFilterSnoopReq once the thread has been started.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
src/nwfilter/nwfilter_dhcpsnoop.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c
b/src/nwfilter/nwfilter_dhcpsnoop.c
index d2a8062..32ca304 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -720,7 +720,10 @@ virNWFilterSnoopReqPut(virNWFilterSnoopReqPtr req)
virNWFilterSnoopLock();
-if (virAtomicIntDecAndTest(req-refctr)) {
+virAtomicIntDecAndTest(req-refctr);
+
+/* make sure it's 0; virAtomitIntDecAndTest may return true on '1' */
+if (virAtomicIntGet(req-refctr) == 0) {
NACK, using two atomic functions, you are making this non-atomic.
between these two atomic operations many things can happen an it's not
what you want I bet. The virAtomicIntDecAndTest() uses
__sync_fetch_and_sub(atomic, 1) and compares it to 1, that's true, but
since it is fetch_and_sub (and not the other way around, the value
being compared to 1 is the value that the atomic had before it was
decremented. That means it returns 1 (true) if and only if the
current value is 0. virAtomicIntDecAndTest() is what you really want
and should use here.
Martin
/*
* delete the request:
* - if we don't find req on the global list anymore
@@ -1605,6 +1608,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr
techdriver,
int tmp;
virThread thread;
virNWFilterVarValuePtr dhcpsrvrs;
+bool threadPuts = false;
virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr);
@@ -1690,6 +1694,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr
techdriver,
/* prevent thread from holding req */
virNWFilterSnoopReqLock(req);
+threadPuts = true;
+
if (virThreadCreate(thread, false, virNWFilterDHCPSnoopThread,
req) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1737,7 +1743,8 @@ exit_rem_ifnametokey:
exit_snoopunlock:
virNWFilterSnoopUnlock();
exit_snoopreqput:
-virNWFilterSnoopReqPut(req);
+if (!threadPuts)
+virNWFilterSnoopReqPut(req);
return -1;
}
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
