Re: [libvirt] [PATCH] trivial libvirt example code
Dave Allan dal...@redhat.com wrote:
...
+static int
+showDomains(virConnectPtr conn)
+{
+int ret = 0, i, numNames, numInactiveDomains, numActiveDomains;
+char **nameList = NULL;
+
+numActiveDomains = virConnectNumOfDomains(conn);
+numInactiveDomains = virConnectNumOfDefinedDomains(conn);
It'd be good to handle numInactiveDomains 0 differently.
Currently it'll probably provoke a failed malloc, below.
Doh--thanks. I missed that those calls could fail.
The warning sign for me was that they were declared to be
of type int. I asked myself why?: for something that's
supposed to count, why allow negative numbers.
+printf(There are %d active and %d inactive domains\n,
+ numActiveDomains, numInactiveDomains);
+
+nameList = malloc(sizeof(char *) * (unsigned int)numInactiveDomains);
...
+if (NULL != nameList) {
+free(nameList);
The test for non-NULL-before-free is unnecessary,
since free is guaranteed to handle NULL properly.
So just call free:
free(nameList);
In fact, if you run make syntax-check before making the
suggested change, it should detect and complain about this code.
Removed. (make syntax-check does not complain, btw)
Ah. thanks for mentioning that.
The script that detects those detects if (expr != NULL) free(expr),
but didn't bother to match the less common case where NULL is first:
if (NULL != expr) free(expr). I've made the upstream source of
that script detect your syntax, too:
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=471cbb075f7
so none of those will slip into libvirt either, once we do the
next gnulib-libvirt sync.
...
I noticed that you're using the git mirror. Good! ;-)
When posting patches, please use git format-patch.
Will do.
That would have made it easier for me to apply and test
your patches. As it is, I didn't do either because
git am FILE didn't work:
$ git am dallan.patch
Applying: trivial libvirt example code
warning: examples/hellolibvirt/hellolibvirt.c has type 100755, expected
100644
error: patch failed: examples/hellolibvirt/hellolibvirt.c:97
error: examples/hellolibvirt/hellolibvirt.c: patch does not apply
Patch failed at 0001 trivial libvirt example code
When you have resolved this problem run git am --resolved.
If you would prefer to skip this patch, instead run git am --skip.
To restore the original branch and stop patching run git am --abort.
Note the warning about permissions on hellolibvirt.c.
You can correct that by running chmod a-x hellolibvirt.c.
The permissions problem is strange--it's 644 in my development tree, and
the patch I sent has:
diff --git a/examples/hellolibvirt/hellolibvirt.c
b/examples/hellolibvirt/hellolibvirt.c
new file mode 100644
What would cause git-am to think it was 755?
I'll investigate if it happens again.
Here are some contribution guidelines that generally make it
easier for maintainers/committers to deal with contributed patches,
(though some parts are specific to git-managed projects):
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob;f=HACKING;hb=HEAD
Good stuff.
When I have a patch like this that people have commented on and I've
modified it slightly in response, what's the best way to re-submit it?
When Rich responded, I submitted both the entire patch with the changes
as well as the changes separately.
Personally I find 2nd-iteration reviews to work best when the
incremental patch is posted with either 1) the preceding
or 2) the squashed/final patch.
Otherwise, I have to apply the preceding patch and the new patch
on separate git branches, then diff those branches to see
the incremental. That's not frustrating and inefficient.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Install schemas into correct location for Solaris
john.le...@sun.com wrote:
Install schemas into correct location for Solaris
Signed-off-by: John Levon john.le...@sun.com
diff --git a/configure.in b/configure.in
--- a/configure.in
+++ b/configure.in
@@ -141,6 +141,10 @@ if test -n $UDEVSETTLE; then
AC_DEFINE_UNQUOTED([UDEVSETTLE],[$UDEVSETTLE],
[Location or name of the udevsettle program])
fi
+
+SCHEMA_DIR=${pkgdatadir}/schemas
+test `uname -s` = SunOS SCHEMA_DIR=${datadir}/lib/xml/rng/libvirt
ACK, but please drop the curly braces on the two lines above.
They just add unnecessary syntax.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] trivial libvirt example code
On Tue, Jan 27, 2009 at 12:05:44AM -0500, Dave Allan wrote: Thanks for taking a look at it. I added a little code to let the user specify a URI on the command line. Do you think it is worth committing? Yes, +1 Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt for rhel-5?
On Wed, Jan 28, 2009 at 01:00:49AM +0100, Farkas Levente wrote: hi, is there any plan to be able to buils libvirt on rhel-5 ie. epel-5? currently all virt and kvm releated packages can be build on epel except libvirt since this dbus problem. This is already fixed in CVS. http://libvirt.org/hg/libvirt-mirror/diff/9e940abe32a1/src/node_device_hal.c Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] avoid printf format-mismatch warnings
autobuild.sh fails like this on at least RHEL5.3:
cc1: warnings being treated as errors
qemud.c: In function 'qemudClientReadBuf':
qemud.c:1470: warning: format '%d' expects type 'int', but argument 7 has
type 'ssize_t'
qemud.c: In function 'qemudClientWriteBuf':
qemud.c:1695: warning: format '%d' expects type 'int', but argument 7 has
type 'ssize_t'
make[2]: *** [libvirtd-qemud.o] Error 1
so I'm about to apply the following fix:
From 162177b69fec410b7d940cf9242cba9b147f0bdb Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Wed, 28 Jan 2009 12:08:26 +0100
Subject: [PATCH] avoid printf format-mismatch warnings
* qemud/qemud.c (qemudClientReadBuf, qemudClientWriteBuf):
Use %lld and a (long long int) cast to print a ssize_t value.
---
qemud/qemud.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/qemud/qemud.c b/qemud/qemud.c
index eb91533..fa5e17d 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -1467,7 +1467,8 @@ static ssize_t qemudClientReadBuf(struct qemud_client
*client,
ssize_t ret;
if (len 0) {
-VIR_ERROR(_(unexpected negative length request %d), len);
+VIR_ERROR(_(unexpected negative length request %lld),
+ (long long int) len);
qemudDispatchClientFailure(client);
return -1;
}
@@ -1692,7 +1693,8 @@ static ssize_t qemudClientWriteBuf(struct qemud_client
*client,
ssize_t ret;
if (len 0) {
-VIR_ERROR(_(unexpected negative length request %d), len);
+VIR_ERROR(_(unexpected negative length request %lld),
+ (long long int) len);
qemudDispatchClientFailure(client);
return -1;
}
--
1.6.1.1.374.g0d9d7
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt for rhel-5?
Daniel P. Berrange wrote: On Wed, Jan 28, 2009 at 01:00:49AM +0100, Farkas Levente wrote: hi, is there any plan to be able to buils libvirt on rhel-5 ie. epel-5? currently all virt and kvm releated packages can be build on epel except libvirt since this dbus problem. This is already fixed in CVS. http://libvirt.org/hg/libvirt-mirror/diff/9e940abe32a1/src/node_device_hal.c thanks. will be there a newer release 0.5.2 (and may be a rawhide src.rpm too) in the near future or it's better to patch 0.5.1? -- Levente Si vis pacem para bellum! -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Install schemas into correct location for Solaris
On Tue, Jan 27, 2009 at 09:23:52PM -0800, john.le...@sun.com wrote:
# HG changeset patch
# User john.le...@sun.com
# Date 1233119659 28800
# Node ID cc848242fa810e8d0126b651849de715c7ef32e4
# Parent f91041f0c607be72c4b5695f081d20716521f6c5
Install schemas into correct location for Solaris
Signed-off-by: John Levon john.le...@sun.com
diff --git a/configure.in b/configure.in
--- a/configure.in
+++ b/configure.in
@@ -141,6 +141,10 @@ if test -n $UDEVSETTLE; then
AC_DEFINE_UNQUOTED([UDEVSETTLE],[$UDEVSETTLE],
[Location or name of the udevsettle program])
fi
+
+SCHEMA_DIR=${pkgdatadir}/schemas
+test `uname -s` = SunOS SCHEMA_DIR=${datadir}/lib/xml/rng/libvirt
+AC_SUBST([SCHEMA_DIR])
I think it is desirable to have a standard location for libvirt
XML schemas that any application can rely on, regardless of OS.
I'd like to avoid apps like virt-manager having to look for RNG
files in a different place on each OS.
At the same time, obviously some OS may have standard locations
where XML parsers prefer to find these things.
So my preference would be to always install in
/usr/share/libvirt/schemas/
*and* then optionally also install the same files into an
OS specific location like your
/usr/share/lib/xml/rng/libvirt
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix misuse of PF_UNIX
On Wed, Jan 28, 2009 at 11:06:03AM +0100, Jim Meyering wrote: john.le...@sun.com wrote: Fix misuse of PF_UNIX PF_UNIX is a protocol familay, not a valid protocol, so isn't suitable for the 3rd socket() argument. Solaris should be ignoring the invalid protocol anyway, but this fix is correct regardless. Signed-off-by: John Levon john.le...@sun.com diff --git a/src/xend_internal.c b/src/xend_internal.c --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -752,7 +752,11 @@ xenDaemonOpen_unix(virConnectPtr conn, c memset(priv-addr, 0, sizeof(priv-addr)); priv-addrfamily = AF_UNIX; -priv-addrprotocol = PF_UNIX; +/* + * This must be zero on Solaris at least for AF_UNIX (which should + * really be PF_UNIX, but doesn't matter). + */ +priv-addrprotocol = 0; priv-addrlen = sizeof(struct sockaddr_un); addr = (struct sockaddr_un *)priv-addr; This looks like a fine change, especially since the general recommendation is to specify protocol as 0. Considering the above and that this is a bug fix, I'd drop the comment altogether. No, please don't drop the comment - the lack of such a warning comment is why I considered it safe to add the PF_UNIX setting in the first place Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix misuse of PF_UNIX
On Tue, Jan 27, 2009 at 05:26:33PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1233105335 28800 # Node ID b3a2537e2f3d5ccb055df1820c112b469a26efdb # Parent 35f5d4f77a3f50cadacf32100fdbd992394f6002 Fix misuse of PF_UNIX PF_UNIX is a protocol familay, not a valid protocol, so isn't suitable for the 3rd socket() argument. Solaris should be ignoring the invalid protocol anyway, but this fix is correct regardless. Signed-off-by: John Levon john.le...@sun.com ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix stderr spewage from docs/examples install
On Mon, Jan 26, 2009 at 05:25:04AM -0800, john.le...@sun.com wrote: # HG changeset patch # User John Levon john.le...@sun.com # Date 1232970961 28800 # Node ID 839f1721d6d9c2cfdbc9327814a4c4a9564ff814 # Parent 26d337992a98a00d98f83247eb113ca3d729d8ef Fix stderr spewage from docs/examples install *.res don't exist, so shouldn't be in the generated Makefile.am Signed-off-by: John Levon john.le...@sun.com ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt for rhel-5?
On Wed, Jan 28, 2009 at 12:17:40PM +0100, Farkas Levente wrote: Daniel P. Berrange wrote: On Wed, Jan 28, 2009 at 01:00:49AM +0100, Farkas Levente wrote: hi, is there any plan to be able to buils libvirt on rhel-5 ie. epel-5? currently all virt and kvm releated packages can be build on epel except libvirt since this dbus problem. This is already fixed in CVS. http://libvirt.org/hg/libvirt-mirror/diff/9e940abe32a1/src/node_device_hal.c thanks. will be there a newer release 0.5.2 (and may be a rawhide src.rpm too) in the near future or it's better to patch 0.5.1? The next update will be 0.6.0, hopefully sometime in the next week. NB, if you're re-packaging libvirt for EPEL-5, be warned that none of the previous upstream releases fully work with RHEL-5 Xen because they are not aware of some significant backported features. In the 0.6.0 release you should make sure you run configure using --with-rhel5-api=yes command line arg to fully support RHEL-5 Xen. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Reset global error object at end of test
Daniel P. Berrange berra...@redhat.com wrote: There statstest test case is expected to raise a number of libvirt errors. Since these are now stored in thread locals, it is expected that this won't be free'd automatically at system shutdown. This patch adds a call to virResetError() at the end to release the memory associated with the stored error object. There is still one block of memory alocated though, which is the thread local error error object itself. We can't free this easily, so I just add a valgrind suppression rule instead. This lets 'make valgrind' pass again Also since the test case doesn't call virInitialize()as a normal app would do, we need to explicitly initialize the thread local storage to ensure things work correctly. valgrind++ ACK. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: Detect SASL library on Solaris
I did a test build of CVS on Solaris and noticed it didn't detect SASL, even though SASL was available. It turns out the library on SOlaris is called libsasl.so, instead of libsasl2.so. It is still API compatible with the Linux one, so there does not seem to be a reason not to use it So this patch extends the configure test to make this work Daniel Index: configure.in === RCS file: /data/cvs/libvirt/configure.in,v retrieving revision 1.202 diff -u -p -u -p -r1.202 configure.in --- configure.in27 Jan 2009 15:29:53 - 1.202 +++ configure.in28 Jan 2009 11:38:25 - @@ -517,18 +517,26 @@ if test x$with_sasl != xno; then fail=1 fi]) if test x$with_sasl != xno ; then -AC_CHECK_LIB([sasl2], [sasl_client_init],[with_sasl=yes],[ - if test x$with_sasl = xcheck ; then +AC_CHECK_LIB([sasl2], [sasl_client_init],[ + SASL_LIBS=$SASL_LIBS -lsasl2 + with_sasl=yes +],[ + AC_CHECK_LIB([sasl], [sasl_client_init],[ +SASL_LIBS=$SASL_LIBS -lsasl +with_sasl=yes + ],[ +if test x$with_sasl = xcheck ; then with_sasl=no - else +else fail=1 - fi]) +fi + ]) +]) fi test $fail = 1 AC_MSG_ERROR([You must install the Cyrus SASL development package in order to compile libvirt]) CFLAGS=$old_cflags LIBS=$old_libs - SASL_LIBS=$SASL_LIBS -lsasl2 if test x$with_sasl = xyes ; then AC_DEFINE_UNQUOTED([HAVE_SASL], 1, [whether Cyrus SASL is available for authentication]) -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fixes for VNC port handling
On Tue, Jan 27, 2009 at 09:23:42PM -0800, john.le...@sun.com wrote:
Fixes for VNC port handling
When parsing sexpr, the VNC port should not be ignored, even when vncunused is
set. Fix the parsing of vncdisplay, which was broken due to strtol() (never
use this function!).
Signed-off-by: John Levon john.le...@sun.com
diff --git a/src/xend_internal.c b/src/xend_internal.c
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -612,7 +612,9 @@ sexpr_get(virConnectPtr xend, const char
*
* convenience function to lookup an int value in the S-Expression
*
- * Returns the value found or 0 if not found (but may not be an error)
+ * Returns the value found or 0 if not found (but may not be an error).
+ * This function suffers from the flaw that zero is both a correct
+ * return value and an error indicator: careful!
*/
static int
sexpr_int(const struct sexpr *sexpr, const char *name)
@@ -2091,15 +2093,16 @@ xenDaemonParseSxprGraphicsNew(virConnect
port = xenStoreDomainGetVNCPort(conn, def-id);
xenUnifiedUnlock(priv);
+// Didn't find port entry in xenstore
if (port == -1) {
-// Didn't find port entry in xenstore
-port = sexpr_int(node, device/vfb/vncdisplay);
+const char *value = sexpr_node(node,
+device/vfb/vncdisplay);
+if (value != NULL)
+port = strtol(value, NULL, 0);
This isn't checking the return value of strtol, so it could have
parsed garbage from XenD's SEXPR. Prefer to use virStrToLong_i()
and initialize port back to -1 upon failure.
-if ((unused STREQ(unused, 1)) || port == -1) {
+if ((unused STREQ(unused, 1)) || port == -1)
graphics-data.vnc.autoport = 1;
-port = -1;
-}
if (port = 0 port 5900)
port += 5900;
The general idea of the patch seems correct. A question about
the test case though - the code above is dealing with correctly
handling the 'device/vfb/vncdisplay' field in the SEXPR, but
your test SEXPR data doesn't have a 'vncdisplay' field:
+(device
+(vfb
+(vncunused 1)
+(keymap en-us)
+(type vnc)
+(uuid 09666ad1-0c94-d79c-1439-99e05394ee51)
+(location localhost:5900)
+)
+)
And I've not seen this 'location' field before - guess that's
something new in XenD we've not handled before.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] proxy: Fix use of uninitalized memory
Rasputin raspu...@email.ru wrote: On short read, members of packet header are checked before actually read. If uninitialized values can pass the test, they can be set to arbitrary values while reading remaining portion of a packet. Buffer overflow is possible. libvirt_proxy is suid-root. diff -urp libvirt-0.5.1/proxy/libvirt_proxy.c libvirt-dev/proxy/libvirt_proxy.c --- libvirt-0.5.1/proxy/libvirt_proxy.c 2008-11-20 08:58:43.0 +0100 +++ libvirt-dev/proxy/libvirt_proxy.c 2009-01-25 12:51:33.0 +0100 @@ -385,7 +385,8 @@ retry: fprintf(stderr, read %d bytes from client %d on socket %d\n, ret, nr, pollInfos[nr].fd); -if ((req-version != PROXY_PROTO_VERSION) || +if ((ret != sizeof(virProxyPacket)) || +(req-version != PROXY_PROTO_VERSION) || (req-len sizeof(virProxyPacket)) || (req-len sizeof(virProxyFullPacket))) goto comm_error; This looks like a good patch. Thanks! -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: rpcgen portability fixes for remote_protocol.c/h
As per previous patches from John for Solaris, we ned a couple more fixes to the generated remote_protocol.c/h files for good portability. Specifically we need s/u_quad_t/uint64_t/g; s/quad_t/int64_t/g; s/xdr_u_quad_t/xdr_uint64_t/g; s/xdr_quad_t/xdr_int64_t/g; s/IXDR_GET_LONG/IXDR_GET_INT32/g; I have finally got around to verifying that this won't change wire ABI on Linux The first two data types int64 and quad_t are all fixed 64-bit ints, so that's safe. And the xdr_quad functions have this in the source strong_alias (xdr_int64_t, xdr_quad_t) strong_alias (xdr_int64_t, xdr_u_quad_t) So that change is no-op. Finally the glibc rpc/xdr.h has #define IXDR_GET_LONG(buf) ((long)IXDR_GET_U_INT32(buf)) So I believe this is all safe. Daniel Index: configure.in === RCS file: /data/cvs/libvirt/configure.in,v retrieving revision 1.202 diff -u -p -r1.202 configure.in --- configure.in27 Jan 2009 15:29:53 - 1.202 +++ configure.in28 Jan 2009 12:30:59 - @@ -92,9 +92,9 @@ AC_CHECK_LIB([intl],[gettext],[]) dnl Do we have rpcgen? AC_PATH_PROG([RPCGEN], [rpcgen], [no]) -AM_CONDITIONAL([RPCGEN], [test x$ac_cv_path_RPCGEN != xno]) +AM_CONDITIONAL([HAVE_RPCGEN], [test x$ac_cv_path_RPCGEN != xno]) dnl Is this GLIBC's buggy rpcgen? -AM_CONDITIONAL([GLIBC_RPCGEN], +AM_CONDITIONAL([HAVE_GLIBC_RPCGEN], [test x$ac_cv_path_RPCGEN != xno $ac_cv_path_RPCGEN -t /dev/null /dev/null 21]) Index: qemud/Makefile.am === RCS file: /data/cvs/libvirt/qemud/Makefile.am,v retrieving revision 1.72 diff -u -p -r1.72 Makefile.am --- qemud/Makefile.am 12 Jan 2009 19:19:22 - 1.72 +++ qemud/Makefile.am 28 Jan 2009 12:31:00 - @@ -1,7 +1,5 @@ ## Process this file with automake to produce Makefile.in -RPCGEN = $(RPCGEN) - DAEMON_SOURCES = \ event.c event.h \ qemud.c qemud.h \ @@ -33,32 +31,34 @@ EXTRA_DIST = \ $(AVAHI_SOURCES)\ $(DAEMON_SOURCES) -if RPCGEN -SUFFIXES = .x -# The subshell ensures that remote_protocol.c ends up -# including config.h before remote_protocol.h. -.x.c: - rm -f $@ $...@-t $...@-t1 $...@-t2 - $(RPCGEN) -c -o $...@-t $ - (echo '#include config.h'; cat $...@-t) $...@-t1 -if GLIBC_RPCGEN - perl -w rpcgen_fix.pl $...@-t1 $...@-t2 - rm $...@-t1 - chmod 444 $...@-t2 - mv $...@-t2 $@ +if HAVE_RPCGEN +# +# Maintainer-only target for re-generating the derived .c/.h source +# files, which are actually derived from the .x file. +# +# For committing protocol changes to CVS, the GLIBC rpcgen *must* +# be used. +# +# Support for non-GLIB rpcgen is here as a convenience for +# non-Linux people needing to test changes during dev. +# +rpcgen: + rm -f rp.c-t rp.h-t rp.c-t1 rp.c-t2 rp.h-t1 + $(RPCGEN) -h -o rp.h-t $(srcdir)/remote_protocol.x + $(RPCGEN) -c -o rp.c-t $(srcdir)/remote_protocol.x +if HAVE_GLIBC_RPCGEN + perl -w $(srcdir)/rpcgen_fix.pl rp.h-t rp.h-t1 + perl -w $(srcdir)/rpcgen_fix.pl rp.c-t rp.c-t1 + (echo '#include config.h'; cat rp.c-t1) rp.c-t2 + chmod 0444 rp.c-t2 rp.h-t1 + mv -f rp.h-t1 $(srcdir)/remote_protocol.h + mv -f rp.c-t2 $(srcdir)/remote_protocol.c + rm -f rp.c-t rp.h-t rp.c-t1 else - chmod 444 $...@-t1 - mv $...@-t1 $@ -endif - -.x.h: - rm -f $@ $...@-t - $(RPCGEN) -h -o $...@-t $ -if GLIBC_RPCGEN - perl -pi -e 's/\t//g' $...@-t + chmod 0444 rp.c-t rp.h-t + mv -f rp.h-t $(srcdir)/remote_protocol.h + mv -f rp.c-t $(srcdir)/remote_protocol.c endif - chmod 444 $...@-t - mv $...@-t $@ endif remote_protocol.c: remote_protocol.h Index: qemud/remote_protocol.c === RCS file: /data/cvs/libvirt/qemud/remote_protocol.c,v retrieving revision 1.27 diff -u -p -r1.27 remote_protocol.c --- qemud/remote_protocol.c 6 Jan 2009 18:32:03 - 1.27 +++ qemud/remote_protocol.c 28 Jan 2009 12:31:00 - @@ -183,7 +183,7 @@ xdr_remote_vcpu_info (XDR *xdrs, remote_ return FALSE; if (!xdr_int (xdrs, objp-state)) return FALSE; - if (!xdr_u_quad_t (xdrs, objp-cpu_time)) + if (!xdr_uint64_t (xdrs, objp-cpu_time)) return FALSE; if (!xdr_int (xdrs, objp-cpu)) return FALSE; @@ -205,11 +205,11 @@ xdr_remote_sched_param_value (XDR *xdrs, return FALSE; break; case VIR_DOMAIN_SCHED_FIELD_LLONG: - if (!xdr_quad_t (xdrs, objp-remote_sched_param_value_u.l)) +
Re: [libvirt] [PATCH] proxy: Fix use of uninitalized memory
Rasputin raspu...@email.ru wrote: On short read, members of packet header are checked before actually read. If uninitialized values can pass the test, they can be set to arbitrary values while reading remaining portion of a packet. Buffer overflow is possible. libvirt_proxy is suid-root. diff -urp libvirt-0.5.1/proxy/libvirt_proxy.c libvirt-dev/proxy/libvirt_proxy.c --- libvirt-0.5.1/proxy/libvirt_proxy.c 2008-11-20 08:58:43.0 +0100 +++ libvirt-dev/proxy/libvirt_proxy.c 2009-01-25 12:51:33.0 +0100 @@ -385,7 +385,8 @@ retry: fprintf(stderr, read %d bytes from client %d on socket %d\n, ret, nr, pollInfos[nr].fd); -if ((req-version != PROXY_PROTO_VERSION) || +if ((ret != sizeof(virProxyPacket)) || +(req-version != PROXY_PROTO_VERSION) || (req-len sizeof(virProxyPacket)) || (req-len sizeof(virProxyFullPacket))) goto comm_error; Thanks again. Here's what I expect to commit. If you would like different attribution, let me know. From 0abc169e868d7b7b5d1edd75aec0e021b1e67c53 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Wed, 28 Jan 2009 14:27:11 +0100 Subject: [PATCH] libvirt_proxy: avoid potential buffer overflow * proxy/libvirt_proxy.c (proxyReadClientSocket): Ensure that we've read an entire virProxyPacket before dereferencing req. Analysis and patch by Rasputin raspu...@email.ru. Details in http://thread.gmane.org/gmane.comp.emulators.libvirt/11459. --- proxy/libvirt_proxy.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index f3d9ede..863dc32 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -2,7 +2,7 @@ * proxy_svr.c: root suid proxy server for Xen access to APIs with no * side effects from unauthenticated clients. * - * Copyright (C) 2006, 2007, 2008 Red Hat, Inc. + * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -382,7 +382,8 @@ retry: fprintf(stderr, read %d bytes from client %d on socket %d\n, ret, nr, pollInfos[nr].fd); -if ((req-version != PROXY_PROTO_VERSION) || +if ((ret != sizeof(virProxyPacket)) || +(req-version != PROXY_PROTO_VERSION) || (req-len sizeof(virProxyPacket)) || (req-len sizeof(virProxyFullPacket))) goto comm_error; -- 1.6.1.1.374.g0d9d7 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] proxy: Fix use of uninitalized memory
Jim Meyering j...@meyering.net wrote: ... Thanks again. Here's what I expect to commit. Committed that. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fixes for VNC port handling
On Wed, Jan 28, 2009 at 11:29:16AM +, Daniel P. Berrange wrote: +device/vfb/vncdisplay); +if (value != NULL) +port = strtol(value, NULL, 0); This isn't checking the return value of strtol, so it could have parsed garbage from XenD's SEXPR. Prefer to use virStrToLong_i() and initialize port back to -1 upon failure. OK. The general idea of the patch seems correct. A question about the test case though - the code above is dealing with correctly handling the 'device/vfb/vncdisplay' field in the SEXPR, but your test SEXPR data doesn't have a 'vncdisplay' field: I screwed up the test. It was working on a live system, but of course there's no xenstore in the test suite, so it wasn't testing what I thought it was. I've modified the test to specify a vncdisplay. +(uuid 09666ad1-0c94-d79c-1439-99e05394ee51) +(location localhost:5900) And I've not seen this 'location' field before - guess that's something new in XenD we've not handled before. Apparently so. However, handling it is strictly pointless: like vncdisplay, it's only useful for static test cases. In all other cases we use the authoritative xenstore results... regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] use virReportOOMError, not VIR_ERR_NO_MEMORY
Daniel P. Berrange berra...@redhat.com wrote:
On Tue, Jan 27, 2009 at 04:59:20PM +0100, Jim Meyering wrote:
Here's the big one:
From 099536470ae2cbe9503ed471d391e403fb2587a4 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Tue, 27 Jan 2009 12:20:06 +0100
Subject: [PATCH 1/3] error-reporting calls using VIR_ERR_NO_MEMORY: use
virReportOOMError instead
* src/uml_conf.c (VIR_FROM_THIS): Define to VIR_FROM_UML.
* src/xs_internal.c (VIR_FROM_THIS): Define to VIR_FROM_XEN.
* src/xml.c (VIR_FROM_THIS): Define to VIR_FROM_XML.
* src/stats_linux.c (VIR_FROM_THIS): Define to VIR_FROM_STATS_LINUX.
* src/datatypes.c (VIR_FROM_THIS): Define to VIR_FROM_NONE.
* src/lxc_conf.c (VIR_FROM_THIS): Define to VIR_FROM_LXC.
* src/libvirt.c (VIR_FROM_THIS): Define to VIR_FROM_NONE.
* src/node_device_conf.c (VIR_FROM_THIS): Define to VIR_FROM_NODEDEV.
* src/openvz_conf.c (VIR_FROM_THIS): Define to VIR_FROM_OPENVZ.
* src/openvz_driver.c (VIR_FROM_THIS): Define to VIR_FROM_OPENVZ.
* src/conf.c (VIR_FROM_THIS): Define to VIR_FROM_CONF.
Note: this loses config_filename:config_lineno diagnostics,
but that's ok.
* src/node_device.c (VIR_FROM_THIS): Define to VIR_FROM_NODEDEV.
* src/sexpr.c (VIR_FROM_THIS): Define to VIR_FROM_SEXPR.
ACK.
After a quick run through the patch I don't see any problems hiding
there - well at least none which weren't already present.
...
We should add a Makefile.maint rule to prohibit use of VIR_ERR_NO_MEMORY
in any file except virterror.c
There were a few more to remove, so here are two patches.
First, an incremental removing the remaining uses of VIR_ERR_NO_MEMORY.
Then a patch adding a syntax-check rule to prevent reintroduction.
Note that the Makefile.maint patch isn't usable as-is;
it requires a preceding patch to sync from coreutils's maint.mk.
And once I did that, a few more syntax-check failures were triggered,
and so I disabled the corresponding rules (temporarily) in another patch.
Both coming up.
FYI, here are the exceptions in .c/.h files:
include/libvirt/virterror.h-typedef enum {
include/libvirt/virterror.h-VIR_ERR_OK = 0,
include/libvirt/virterror.h-VIR_ERR_INTERNAL_ERROR, /* internal error */
include/libvirt/virterror.h:VIR_ERR_NO_MEMORY, /* memory allocation
failure */
include/libvirt/virterror.h-VIR_ERR_NO_SUPPORT, /* no support for this
function */
include/libvirt/virterror.h-VIR_ERR_UNKNOWN_HOST,/* could not resolve
hostname */
include/libvirt/virterror.h-VIR_ERR_NO_CONNECT, /* can't connect to
hypervisor */
--
qemud/remote.c-remoteDispatchOOMError (remote_error *rerr)
qemud/remote.c-{
qemud/remote.c-remoteDispatchStringError(rerr,
qemud/remote.c: VIR_ERR_NO_MEMORY,
qemud/remote.c- NULL);
qemud/remote.c-}
qemud/remote.c-
--
src/virterror.c-else
src/virterror.c- errmsg = _(internal error);
src/virterror.c-break;
src/virterror.c:case VIR_ERR_NO_MEMORY:
src/virterror.c-errmsg = _(out of memory);
src/virterror.c-break;
src/virterror.c-case VIR_ERR_NO_SUPPORT:
--
src/virterror.c-{
src/virterror.c-const char *virerr;
src/virterror.c-
src/virterror.c:virerr = virErrorMsg(VIR_ERR_NO_MEMORY, NULL);
src/virterror.c:virRaiseError(conn, NULL, NULL, domcode, VIR_ERR_NO_MEMORY,
VIR_ERR_ERROR,
src/virterror.c- virerr, NULL, NULL, -1, -1, virerr, NULL);
src/virterror.c-}
From afa312df336037fcfc166939f3aa37513da4e41d Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Tue, 27 Jan 2009 21:37:13 +0100
Subject: [PATCH 4/8] FIXME: to-be-squashed: VIR_ERR_NO_MEMORY -
virReportOOMError
---
src/remote_internal.c |8 ++--
src/sexpr.c |3 +--
src/storage_backend_fs.c |7 +++
src/storage_backend_logical.c |4 ++--
4 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 5c2e705..0fb5d87 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -4957,10 +4957,7 @@ static char *addrToString(struct sockaddr_storage *sa,
socklen_t salen)
}
if (VIR_ALLOC_N(addr, strlen(host) + 1 + strlen(port) + 1) 0) {
-virRaiseError (NULL, NULL, NULL, VIR_FROM_REMOTE,
- VIR_ERR_NO_MEMORY, VIR_ERR_ERROR,
- NULL, NULL, NULL, 0, 0,
- address);
+virReportOOMError (NULL);
return NULL;
}
@@ -6315,8 +6312,7 @@ call (virConnectPtr conn, struct private_data *priv,
ret_filter, ret);
if (!thiscall) {
-error (flags REMOTE_CALL_IN_OPEN ? NULL : conn,
- VIR_ERR_NO_MEMORY, NULL);
+virReportOOMError (flags REMOTE_CALL_IN_OPEN ? NULL : conn);
return -1;
}
diff --git a/src/sexpr.c b/src/sexpr.c
index 7450c79..bc82d1f 100644
--- a/src/sexpr.c
+++ b/src/sexpr.c
@@
[libvirt] [PATCH 6/8] Makefile.maint: sync from coreutils
You can ignore the patch numbering.
There was yet another one adjusting po/POTFILES.in
that I'm not bothering to post.
Probably not worth reviewing.
Some of it is just factorization.
Other bits reflect removal of coreutils-specific things (they're
migrating into coreutils' cfg.mk (was called Makefile.cfg).
On the other hand, a few are introduction of cu-specific bits,
but that are enabled only if certain coreutils-specific files
are present. This is to make later merges easier.
I've made sure that make syntax-check still works.
From 19d1ea009c2d1235a271915e55eac9e170e3530c Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Wed, 28 Jan 2009 16:45:18 +0100
Subject: [PATCH 6/8] * Makefile.maint: sync from coreutils
---
Makefile.maint | 426 +++
1 files changed, 240 insertions(+), 186 deletions(-)
diff --git a/Makefile.maint b/Makefile.maint
index 204370d..c50d100 100644
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -25,9 +25,7 @@ syntax-check-rules := $(shell sed -n
's/^\(sc_[a-zA-Z0-9_-]*\):.*/\1/p' \
.PHONY: $(syntax-check-rules)
local-checks-available = \
- po-check copyright-check m4-check author_mark_check \
- patch-check strftime-check $(syntax-check-rules) \
- makefile_path_separator_check \
+ patch-check $(syntax-check-rules) \
makefile-check check-AUTHORS
.PHONY: $(local-checks-available)
@@ -45,6 +43,17 @@ syntax-check: $(local-check)
# exit 1; } || :
# FIXME: don't allow `#include .strings\.h' anywhere
+# There are many rules below that prohibit constructs in this package.
+# If the offending construct can be matched with a grep-E-style regexp,
+# use this macro. The shell variables re and msg must be defined.
+define _prohibit_regexp
+ dummy=; : so we do not need a semicolon before each use \
+ test x$$re != x || { echo '$(ME): re not defined' 12; exit 1; }; \
+ test x$$msg != x || { echo '$(ME): msg not defined' 12; exit 1; };\
+ grep -nE $$re $$($(VC_LIST_EXCEPT)) \
+{ echo '$(ME): '$$msg 12; exit 1; } || :
+endef
+
sc_avoid_if_before_free:
@$(srcdir)/build-aux/useless-if-before-free \
$(useless_free_options) \
@@ -64,40 +73,29 @@ sc_avoid_write:
fi
sc_cast_of_argument_to_free:
- @grep -nE '\free *\( *\(' $$($(VC_LIST_EXCEPT)) \
- { echo '$(ME): don'\''t cast free argument' 12; \
- exit 1; } || :
+ @re='\free *\( *\(' msg='don'\''t cast free argument' \
+ $(_prohibit_regexp)
sc_cast_of_x_alloc_return_value:
- @grep -nE '\*\) *x(m|c|re)alloc\' $$($(VC_LIST_EXCEPT)) \
- { echo '$(ME): don'\''t cast x*alloc return value' 12; \
- exit 1; } || :
+ @re='\*\) *x(m|c|re)alloc\'\
+ msg='don'\''t cast x*alloc return value'\
+ $(_prohibit_regexp)
sc_cast_of_alloca_return_value:
- @grep -nE '\*\) *alloca\' $$($(VC_LIST_EXCEPT)) \
- { echo '$(ME): don'\''t cast alloca return value' 12; \
- exit 1; } || :
+ @re='\*\) *alloca\' msg='don'\''t cast alloca return value'\
+ $(_prohibit_regexp)
sc_space_tab:
- @grep -n '[ ] ' $$($(VC_LIST_EXCEPT)) \
- { echo '$(ME): found SPACE-TAB sequence; remove the SPACE'\
- 12; exit 1; } || :
-
-cvs_keywords = \
- Author|Date|Header|Id|Name|Locker|Log|RCSfile|Revision|Source|State
-
-sc_prohibit_cvs_keyword:
- @grep -nE '\$$($(cvs_keywords))\$$' $$($(VC_LIST_EXCEPT)) \
- { echo '$(ME): do not use CVS keyword expansion ' \
- 12; exit 1; } || :
+ @re='[ ]' msg='found SPACE-TAB sequence; remove the SPACE' \
+ $(_prohibit_regexp)
# Don't use *scanf or the old ato* functions in `real' code.
# They provide no error checking mechanism.
# Instead, use strto* functions.
sc_prohibit_atoi_atof:
- @grep -nE '\([fs]?scanf|ato([filq]|ll))\' $$($(VC_LIST_EXCEPT)) \
- { echo '$(ME): do not use *scan''f, ato''f, ato''i, ato''l, ato''ll,
ato''q, or ss''canf' \
- 12; exit 1; } || :
+ @re='\([fs]?scanf|ato([filq]|ll))\' \
+ msg='do not use *scan''f, ato''f, ato''i, ato''l, ato''ll or ato''q' \
+ $(_prohibit_regexp)
# Use STREQ rather than comparing strcmp == 0, or != 0.
# Similarly, use STREQLEN or STRPREFIX rather than strncmp.
@@ -110,8 +108,9 @@ sc_prohibit_strcmp:
# Use virAsprintf rather than a'sprintf since *strp is undefined on error.
sc_prohibit_asprintf:
- @grep -nE '\[a]sprintf\' $$($(VC_LIST_EXCEPT)) \
- { echo '$(ME): use virAsprintf, not a'sprintf 12; exit 1; } || :
+ @re='\[a]sprintf\'\
+ msg='use virAsprintf, not
[libvirt] PATCH: Misc Xen driver bug fixes
Testing current CVS on RHEL-5 host exposed a number of problems in the
Xen driver
- When opening the remote driver for the network / storage / nodedev
APIs, we had missed the initialization of several fields, resulting
in a crash
- Reporting of errors from the remote driver was leaking memory, due
to missing xdr_free call.
- Small possibility of de-referencing NULL when handling an error
from the server, if no error message were provided
- Mistakenly tries to activate Xen INotify driver when non-root, even
though the directories it needs to monitor are chmod 0700 root.root
- Gratuitous reporting of failure to connect to XenD's TCP port when
running non-root, even though the proxy / remote driver will suceed
- Bad return values from the xenDaemonOpen() method
- Double free in the new xenStoreDoListDomains() method probably due
to merge error
The only really intreresting bit of the patch is for the first issue
in the remote driver. I've basically pulled out alot of duplicated
code into a couple of helper methods, so make these missing initialization
less likely in future.
remote_internal.c | 153 +-
xen_unified.c | 12 ++--
xend_internal.c | 50 ++---
xs_internal.c |2
4 files changed, 97 insertions(+), 120 deletions(-)
Daniel
Index: src/remote_internal.c
===
RCS file: /data/cvs/libvirt/src/remote_internal.c,v
retrieving revision 1.130
diff -u -p -u -p -r1.130 remote_internal.c
--- src/remote_internal.c 28 Jan 2009 16:14:24 - 1.130
+++ src/remote_internal.c 28 Jan 2009 16:24:20 -
@@ -892,31 +892,70 @@ doRemoteOpen (virConnectPtr conn,
goto cleanup;
}
-static virDrvOpenStatus
-remoteOpen (virConnectPtr conn,
-virConnectAuthPtr auth,
-int flags)
+static struct private_data *
+remoteAllocPrivateData(virConnectPtr conn)
{
struct private_data *priv;
-int ret, rflags = 0;
-
-if (inside_daemon)
-return VIR_DRV_OPEN_DECLINED;
-
if (VIR_ALLOC(priv) 0) {
-error (conn, VIR_ERR_NO_MEMORY, _(struct private_data));
-return VIR_DRV_OPEN_ERROR;
+virReportOOMError(conn);
+return NULL;
}
if (virMutexInit(priv-lock) 0) {
error(conn, VIR_ERR_INTERNAL_ERROR,
_(cannot initialize mutex));
VIR_FREE(priv);
-return VIR_DRV_OPEN_ERROR;
+return NULL;
}
remoteDriverLock(priv);
priv-localUses = 1;
priv-watch = -1;
+priv-sock = -1;
+
+return priv;
+}
+
+static int
+remoteOpenSecondaryDriver(virConnectPtr conn,
+ virConnectAuthPtr auth,
+ int flags,
+ struct private_data **priv)
+{
+int ret;
+int rflags = 0;
+
+if (!((*priv) = remoteAllocPrivateData(conn)))
+return VIR_DRV_OPEN_ERROR;
+
+if (flags VIR_CONNECT_RO)
+rflags |= VIR_DRV_OPEN_REMOTE_RO;
+rflags |= VIR_DRV_OPEN_REMOTE_UNIX;
+
+ret = doRemoteOpen(conn, *priv, auth, rflags);
+if (ret != VIR_DRV_OPEN_SUCCESS) {
+remoteDriverUnlock(*priv);
+VIR_FREE(*priv);
+} else {
+(*priv)-localUses = 1;
+remoteDriverUnlock(*priv);
+}
+
+return ret;
+}
+
+static virDrvOpenStatus
+remoteOpen (virConnectPtr conn,
+virConnectAuthPtr auth,
+int flags)
+{
+struct private_data *priv;
+int ret, rflags = 0;
+
+if (inside_daemon)
+return VIR_DRV_OPEN_DECLINED;
+
+if (!(priv = remoteAllocPrivateData(conn)))
+return VIR_DRV_OPEN_ERROR;
if (flags VIR_CONNECT_RO)
rflags |= VIR_DRV_OPEN_REMOTE_RO;
@@ -971,7 +1010,6 @@ remoteOpen (virConnectPtr conn,
#endif
}
-priv-sock = -1;
ret = doRemoteOpen(conn, priv, auth, rflags);
if (ret != VIR_DRV_OPEN_SUCCESS) {
conn-privateData = NULL;
@@ -3085,30 +3123,13 @@ remoteNetworkOpen (virConnectPtr conn,
* which doesn't have its own impl of the network APIs.
*/
struct private_data *priv;
-int ret, rflags = 0;
-if (VIR_ALLOC(priv) 0) {
-error (conn, VIR_ERR_NO_MEMORY, _(struct private_data));
-return VIR_DRV_OPEN_ERROR;
-}
-if (virMutexInit(priv-lock) 0) {
-error(conn, VIR_ERR_INTERNAL_ERROR,
- _(cannot initialize mutex));
-VIR_FREE(priv);
-return VIR_DRV_OPEN_ERROR;
-}
-if (flags VIR_CONNECT_RO)
-rflags |= VIR_DRV_OPEN_REMOTE_RO;
-rflags |= VIR_DRV_OPEN_REMOTE_UNIX;
-
-priv-sock = -1;
-ret = doRemoteOpen(conn, priv, auth, rflags);
-if (ret != VIR_DRV_OPEN_SUCCESS) {
-conn-networkPrivateData = NULL;
-VIR_FREE(priv);
-} else {
-priv-localUses = 1;
+int ret;
+ret =
Re: [libvirt] PATCH: Misc Xen driver bug fixes
Daniel P. Berrange berra...@redhat.com wrote: Testing current CVS on RHEL-5 host exposed a number of problems in the Xen driver - When opening the remote driver for the network / storage / nodedev APIs, we had missed the initialization of several fields, resulting in a crash - Reporting of errors from the remote driver was leaking memory, due to missing xdr_free call. - Small possibility of de-referencing NULL when handling an error from the server, if no error message were provided - Mistakenly tries to activate Xen INotify driver when non-root, even though the directories it needs to monitor are chmod 0700 root.root - Gratuitous reporting of failure to connect to XenD's TCP port when running non-root, even though the proxy / remote driver will suceed - Bad return values from the xenDaemonOpen() method - Double free in the new xenStoreDoListDomains() method probably due to merge error The only really intreresting bit of the patch is for the first issue in the remote driver. I've basically pulled out alot of duplicated code into a couple of helper methods, so make these missing initialization less likely in future. remote_internal.c | 153 +- xen_unified.c | 12 ++-- xend_internal.c | 50 ++--- xs_internal.c |2 4 files changed, 97 insertions(+), 120 deletions(-) ACK. It all looks good, but for the fact that I see it'll cause at least one conflict for me ;-) -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fixes for VNC port handling
# HG changeset patch
# User john.le...@sun.com
# Date 1233162577 28800
# Node ID 6ab030d99574ee01193e92846eb7c9b41bfcd149
# Parent 5a0860d81ed44d5f189788fc340f975517595160
Fixes for VNC port handling
When parsing sexpr, the VNC port should not be ignored, even when vncunused is
set. Fix the parsing of vncdisplay, which was broken due to strtol() (never
use this function!).
Signed-off-by: John Levon john.le...@sun.com
diff --git a/src/util.c b/src/util.c
--- a/src/util.c
+++ b/src/util.c
@@ -1041,6 +1041,7 @@ cleanup:
return rc;
}
+#endif /* PROXY */
/* Like strtol, but produce an int result, and check more carefully.
@@ -1123,7 +1124,6 @@ virStrToLong_ull(char const *s, char **e
*result = val;
return 0;
}
-#endif /* PROXY */
/**
* virSkipSpaces:
diff --git a/src/xend_internal.c b/src/xend_internal.c
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -612,7 +612,9 @@ sexpr_get(virConnectPtr xend, const char
*
* convenience function to lookup an int value in the S-Expression
*
- * Returns the value found or 0 if not found (but may not be an error)
+ * Returns the value found or 0 if not found (but may not be an error).
+ * This function suffers from the flaw that zero is both a correct
+ * return value and an error indicator: careful!
*/
static int
sexpr_int(const struct sexpr *sexpr, const char *name)
@@ -2091,15 +2093,16 @@ xenDaemonParseSxprGraphicsNew(virConnect
port = xenStoreDomainGetVNCPort(conn, def-id);
xenUnifiedUnlock(priv);
+// Didn't find port entry in xenstore
if (port == -1) {
-// Didn't find port entry in xenstore
-port = sexpr_int(node, device/vfb/vncdisplay);
+const char *str = sexpr_node(node,
device/vfb/vncdisplay);
+int val;
+if (str != NULL virStrToLong_i(str, NULL, 0, val) == 0)
+port = val;
}
-if ((unused STREQ(unused, 1)) || port == -1) {
+if ((unused STREQ(unused, 1)) || port == -1)
graphics-data.vnc.autoport = 1;
-port = -1;
-}
if (port = 0 port 5900)
port += 5900;
diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.sexpr
b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.sexpr
new file mode 100644
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.sexpr
@@ -0,0 +1,86 @@
+(domain
+(domid 21)
+(on_crash destroy)
+(uuid e0c172e6-4ad8-7353-0ece-515d2f181365)
+(bootloader_args )
+(vcpus 1)
+(name domu-224)
+(on_poweroff destroy)
+(on_reboot destroy)
+(bootloader )
+(maxmem 512)
+(memory 512)
+(shadow_memory 5)
+(cpu_weight 256)
+(cpu_cap 0)
+(features )
+(on_xend_start ignore)
+(on_xend_stop shutdown)
+(start_time 1233108538.42)
+(cpu_time 907.159661051)
+(online_vcpus 1)
+(image
+(hvm
+(kernel /usr/lib/xen/boot/hvmloader)
+(boot d)
+(device_model /usr/lib/xen/bin/qemu-dm)
+(keymap en-us)
+(localtime 1)
+(pae 1)
+(serial pty)
+(usb 1)
+(usbdevice tablet)
+(notes (SUSPEND_CANCEL 1))
+)
+)
+(status 2)
+(state r-)
+(store_mfn 131070)
+(device
+(vif
+(bridge e1000g0)
+(mac 00:16:3e:1b:e8:18)
+(script vif-vnic)
+(uuid 7da8c614-018b-dc87-6bfc-a296a95bca4f)
+(backend 0)
+)
+)
+(device
+(vbd
+(uname phy:/iscsi/winxp)
+(uuid 65e19258-f4a2-a9ff-3b31-469ceaf4ec8d)
+(mode w)
+(dev hda:disk)
+(backend 0)
+(bootable 1)
+)
+)
+(device
+(vbd
+(uname file:/net/heaped/export/netimage/windows/xp-sp2-vol.iso)
+(uuid 87d9383b-f0ad-11a4-d668-b965f55edc3f)
+(mode r)
+(dev hdc:cdrom)
+(backend 0)
+(bootable 0)
+)
+)
+(device (vkbd (backend 0)))
+(device
+(vfb
+(vncdisplay 25)
+(vncunused 1)
+(keymap en-us)
+(type vnc)
+(uuid 09666ad1-0c94-d79c-1439-99e05394ee51)
+(location localhost:5925)
+)
+)
+(device
+(console
+(protocol vt100)
+(location 3)
+(uuid cabfc0f5-1c9c-0e6f-aaa8-9974262aff66)
+)
+)
+)
diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml
b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml
new file mode 100644
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml
@@ -0,0 +1,48 @@
+domain type='xen' id='21'
+ namedomu-224/name
+ uuide0c172e6-4ad8-7353-0ece-515d2f181365/uuid
+ memory524288/memory
+
Re: [libvirt] [PATCH 6/8] Makefile.maint: sync from coreutils
On Wed, Jan 28, 2009 at 05:12:16PM +0100, Jim Meyering wrote: You can ignore the patch numbering. There was yet another one adjusting po/POTFILES.in that I'm not bothering to post. Probably not worth reviewing. Some of it is just factorization. Other bits reflect removal of coreutils-specific things (they're migrating into coreutils' cfg.mk (was called Makefile.cfg). On the other hand, a few are introduction of cu-specific bits, but that are enabled only if certain coreutils-specific files are present. This is to make later merges easier. I've made sure that make syntax-check still works. ACK If you're taking feature requests, one thing I'd like for the syntax-check target is for it to print out the name of each check it is running. We've got quite alot of checks and so it just sits there for a long time using 100% cpu and with no feedback as to what its doing. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] setting up dnsmasq options for PXE boot
On Thu, 2009-01-22 at 15:20 +, Daniel P. Berrange wrote: On Thu, Jan 22, 2009 at 03:12:39PM +, Richard W.M. Jones wrote: On Tue, Jan 20, 2009 at 02:05:26PM +0300, Dmitry Guryanov wrote: But for working PXE boot it should have also something like --dhcp-boot=pxelinux.0,itchy,192.168.107.1 This isn't supported by libvirt at the moment, but it would great to have a patch which enabled this. PXE-booting using dnsmasq is very reliable, and there's no particular reason why we shouldn't support it. Well the virtual network stuff is local to the host only, and so it is even more reliable if you just boot the guest in question from the kernel+initrd directly, instead indirectly giving it the kernel+ initrd via a dodgy PXE server. We have a hammer, but I don't think we should use it in this case. When you NAT the virtual network to the outside world (like Dmitry does), there's no reason why the TFTP part of a PXE boot should not work. And if you're on a laptop, connected to a network with a working TFTP server, I don't see why you should have to screw around with kernel +initrd. I always felt it's a mistake to work so hard to hide the actual dnsmasq config from users - it would be much more extensible if we plonked a config file into /etc/dnsmasq.d David -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Support VNC password for QEMU guests
Daniel P. Berrange berra...@redhat.com wrote: On Tue, Jan 20, 2009 at 11:08:56PM +, Daniel P. Berrange wrote: This patch adds support for using the monitor interface to set the VNC password (qemu) change vnc password Password: A minor tricky thing is that we can't just send the command and password all in one go, we must wait for the 'Password' prompt before sending the password. When doing this I noticed that virsh dumpxml has no way to request a secure XML dump (required to see the password element), nor did the virsh edit command set the SECURE or INACTIVE flags when changing the XML. qemu_conf.c | 45 --- qemu_driver.c | 112 -- virsh.c | 30 ++- 3 files changed, 131 insertions(+), 56 deletions(-) ... +int flags = 0; +int inactive = vshCommandOptBool(cmd, inactive); +int secure = vshCommandOptBool(cmd, secure); + +if (inactive) +flags |= VIR_DOMAIN_XML_INACTIVE; +if(secure) +flags |= VIR_DOMAIN_XML_SECURE; ACK. My only reservation is that this new --secure option currently means also dump sensitive info (passwords), which is sometimes _in_secure. So how about naming it --all instead? -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] setting up dnsmasq options for PXE boot
On Wed, Jan 28, 2009 at 06:24:17PM +, David Lutterkort wrote: I always felt it's a mistake to work so hard to hide the actual dnsmasq config from users - it would be much more extensible if we plonked a config file into /etc/dnsmasq.d Wouldn't necessarily go as far as adding a config file to /etc/dnsmasq.d, but in general terms +1. We're using dnsmasq. It's an excellent little program with a good license. I don't think we should be afraid to offer specific dnsmasq features. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Fix remote driver RPC recv handling
Daniel P. Berrange berra...@redhat.com wrote:
I noticed a odd error message randomly appearing from virsh after all
commands had been run
# virsh dominfo VirtTest
Id: -
Name: VirtTest
UUID: 82038f2a-1344-aaf7-1a85-2a7250be2076
OS Type:hvm
State: shut off
CPU(s): 3
Max memory: 256000 kB
Used memory:256000 kB
Autostart: disable
libvir: Remote error : server closed connection
It turns out that inthe for(;;) loop where I'm procesing incoming data,
it was forgetting to break out of the loop when a completed RPC call
had been received. Most of the time this wasn't problem, because there'd
rarely be more data following, so it'd get EAGAIN next iteration quit
the loop. When shutting down though, you'd occassionally see the SIGHUP
from read() before you get an EAGAIN, causing this error to be raised
even though the RPC call had been successfully received.
In addition, if there was a 2nd RPC reply already pending, we'd read and
loose some of its data. Though I never saw this happen, it is a definite
theoretical possibility, particularly over a TCP link with bad latency
or fragementation or bursty traffic.
Daniel
diff -r e582072116a3 src/remote_internal.c
--- a/src/remote_internal.c Mon Jan 26 16:21:42 2009 +
+++ b/src/remote_internal.c Mon Jan 26 17:02:15 2009 +
@@ -6135,12 +6135,27 @@ processCallRecv(virConnectPtr conn, stru
if (priv-bufferOffset == priv-bufferLength) {
if (priv-bufferOffset == 4) {
ret = processCallRecvLen(conn, priv, in_open);
+if (ret 0)
+return -1;
This part is no net change. Just hoisting the test+return
that used to follow both calls.
+/*
+ * We'll carry on around the loop to immediately
+ * process the message body, because it has probably
+ * already arrived. Worst case, we'll get EAGAIN on
+ * next iteration.
+ */
} else {
ret = processCallRecvMsg(conn, priv, in_open);
priv-bufferOffset = priv-bufferLength = 0;
-}
-if (ret 0)
-return -1;
+/*
+ * We've completed one call, so return even
+ * though there might still be more data on
+ * the wire. We need to actually let the caller
+ * deal with this arrived message to keep good
+ * response, and also to correctly handle EOF.
+ */
+return ret;
This seems like a good idea for all the reasons you give.
Suggestions, while you're in the area:
- s/EGAIN/EAGAIN/ in a comment just above, in the same function.
- less important: I'd move the declaration of ret down into the
while loop, since it's used only therein. Save 2 lines (yeah, yeah,
out of 6800+ -- gotta start somewhere)
ACK
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: rpcgen portability fixes for remote_protocol.c/h
Daniel P. Berrange berra...@redhat.com wrote:
As per previous patches from John for Solaris, we ned a couple more fixes
to the generated remote_protocol.c/h files for good portability.
Specifically we need
s/u_quad_t/uint64_t/g;
s/quad_t/int64_t/g;
s/xdr_u_quad_t/xdr_uint64_t/g;
s/xdr_quad_t/xdr_int64_t/g;
s/IXDR_GET_LONG/IXDR_GET_INT32/g;
I have finally got around to verifying that this won't change wire
ABI on Linux
The first two data types int64 and quad_t are all fixed 64-bit ints,
so that's safe.
And the xdr_quad functions have this in the source
strong_alias (xdr_int64_t, xdr_quad_t)
strong_alias (xdr_int64_t, xdr_u_quad_t)
So that change is no-op.
Finally the glibc rpc/xdr.h has
#define IXDR_GET_LONG(buf) ((long)IXDR_GET_U_INT32(buf))
So I believe this is all safe.
I think so too.
I've double-checked the .[ch] changes by running this
and comparing; no diffs:
perl -pi -e
's/\bu_quad_t\b/uint64_t/g;s/\bquad_t\b/int64_t/g;s/\bxdr_u_quad_t\b/xdr_uint64_t/g;s/\bxdr_quad_t\b/xdr_int64_t/g;s/\bIXDR_GET_LONG\b/IXDR_GET_INT32/g'
qemud/remote_protocol.[ch]
The configure.in and Makefile changes looked fine, too.
Index: qemud/rpcgen_fix.pl
===
RCS file: /data/cvs/libvirt/qemud/rpcgen_fix.pl,v
retrieving revision 1.4
diff -u -p -r1.4 rpcgen_fix.pl
--- qemud/rpcgen_fix.pl 6 Jan 2009 18:32:03 - 1.4
+++ qemud/rpcgen_fix.pl 28 Jan 2009 12:31:00 -
@@ -26,6 +26,14 @@ while () {
s/\t//g;
+# Portability for Solaris RPC
+s/u_quad_t/uint64_t/g;
+s/quad_t/int64_t/g;
+s/xdr_u_quad_t/xdr_uint64_t/g;
+s/xdr_quad_t/xdr_int64_t/g;
+s/IXDR_GET_LONG/IXDR_GET_INT32/g;
+s,#include \./remote_protocol\.h,#include remote_protocol.h,;
+
if (m/^}/) {
$in_function = 0;
You might want to bracket each LHS with \b...\b, just in case.
s/\bu_quad_t\b/uint64_t/g;
s/\bquad_t\b/int64_t/g;
s/\bxdr_u_quad_t\b/xdr_uint64_t/g;
s/\bxdr_quad_t\b/xdr_int64_t/g;
s/\bIXDR_GET_LONG\b/IXDR_GET_INT32/g;
ACK.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix floppy definition for HVM guests
# HG changeset patch
# User john.le...@sun.com
# Date 1233177786 28800
# Node ID 2b9283e83c20ca2a18f2cafcb963ce254e11dbb4
# Parent 9a9bd34ec485ebaf4c3861bdfffd2953fd91a1fc
Fix floppy definition for HVM guests
Code was missing to define floppy disks for Xen HVM guests. Refuse to
attach disks that aren't supported by direct attach.
Signed-off-by: John Levon john.le...@sun.com
diff --git a/src/xend_internal.c b/src/xend_internal.c
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -5018,14 +5018,26 @@ xenDaemonFormatSxprDisk(virConnectPtr co
* under the hvm (image (os)) block
*/
if (hvm
-def-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+def-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+if (isAttach) {
+virXendError(conn, VIR_ERR_INVALID_ARG,
+ _(Cannot directly attach floppy %s), def-src);
+return -1;
+}
return 0;
+}
/* Xend = 3.0.2 doesn't include cdrom config here */
if (hvm
def-device == VIR_DOMAIN_DISK_DEVICE_CDROM
-xendConfigVersion == 1)
+xendConfigVersion == 1) {
+if (isAttach) {
+virXendError(conn, VIR_ERR_INVALID_ARG,
+ _(Cannot directly attach CDROM %s), def-src);
+return -1;
+}
return 0;
+}
if (!isAttach)
virBufferAddLit(buf, (device );
@@ -5374,17 +5386,29 @@ xenDaemonFormatSxpr(virConnectPtr conn,
}
virBufferVSprintf(buf, (boot %s), bootorder);
-/* get the cdrom device file */
-/* Only XenD = 3.0.2 wants cdrom config here */
-if (xendConfigVersion == 1) {
-for (i = 0 ; i def-ndisks ; i++) {
-if (def-disks[i]-type == VIR_DOMAIN_DISK_DEVICE_CDROM
-STREQ(def-disks[i]-dst, hdc)
-def-disks[i]-src) {
-virBufferVSprintf(buf, (cdrom '%s'),
- def-disks[i]-src);
+/* some disk devices are defined here */
+for (i = 0 ; i def-ndisks ; i++) {
+switch (def-disks[i]-device) {
+case VIR_DOMAIN_DISK_DEVICE_CDROM:
+/* Only xend = 3.0.2 wants cdrom config here */
+if (xendConfigVersion != 1)
break;
-}
+if (!STREQ(def-disks[i]-dst, hdc) ||
+def-disks[i]-src == NULL)
+break;
+
+virBufferVSprintf(buf, (cdrom '%s'),
+ def-disks[i]-src);
+break;
+
+case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+/* all xend versions define floppies here */
+virBufferVSprintf(buf, (%s '%s'), def-disks[i]-dst,
+def-disks[i]-src);
+break;
+
+default:
+break;
}
}
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Support VNC password for QEMU guests
On Wed, Jan 28, 2009 at 07:46:34PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: On Tue, Jan 20, 2009 at 11:08:56PM +, Daniel P. Berrange wrote: This patch adds support for using the monitor interface to set the VNC password (qemu) change vnc password Password: A minor tricky thing is that we can't just send the command and password all in one go, we must wait for the 'Password' prompt before sending the password. When doing this I noticed that virsh dumpxml has no way to request a secure XML dump (required to see the password element), nor did the virsh edit command set the SECURE or INACTIVE flags when changing the XML. qemu_conf.c | 45 --- qemu_driver.c | 112 -- virsh.c | 30 ++- 3 files changed, 131 insertions(+), 56 deletions(-) ... +int flags = 0; +int inactive = vshCommandOptBool(cmd, inactive); +int secure = vshCommandOptBool(cmd, secure); + +if (inactive) +flags |= VIR_DOMAIN_XML_INACTIVE; +if(secure) +flags |= VIR_DOMAIN_XML_SECURE; ACK. My only reservation is that this new --secure option currently means also dump sensitive info (passwords), which is sometimes _in_secure. So how about naming it --all instead? How about --security-info ? I think --all is probably a little too generic a term Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Solaris PV is localtime. Allow this setting through the conversions
# HG changeset patch
# User john.le...@sun.com
# Date 1233198416 28800
# Node ID 76d670ab6ea5476be96441ea88af94644c985201
# Parent f0247b9f4da199c01412d5bd0f9d7d888cbb84de
Solaris PV is localtime. Allow this setting through the conversions.
Signed-off-by: John Levon john.le...@sun.com
diff --git a/src/xend_internal.c b/src/xend_internal.c
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -2272,10 +2272,12 @@ xenDaemonParseSxpr(virConnectPtr conn,
def-features |= (1 VIR_DOMAIN_FEATURE_APIC);
if (sexpr_int(root, domain/image/hvm/pae))
def-features |= (1 VIR_DOMAIN_FEATURE_PAE);
-
-if (sexpr_int(root, domain/image/hvm/localtime))
+}
+
+ if (sexpr_int(root, domain/localtime) ||
+ sexpr_int(root, domain/image/hvm/localtime) ||
+ sexpr_int(root, domain/image/linux/localtime))
def-localtime = 1;
-}
if (sexpr_node_copy(root, hvm ?
domain/image/hvm/device_model :
@@ -5338,6 +5340,14 @@ xenDaemonFormatSxpr(virConnectPtr conn,
}
virBufferVSprintf(buf, (on_crash '%s'), tmp);
+/*
+ * xend puts localtime in image/{hvm,linux}/, but is clearly
+ * bootloader independent. As a simple workaround, we define it
+ * here - xend happily picks it up from here.
+ */
+if (def-localtime)
+virBufferAddLit(buf, (localtime 1));
+
if (!def-os.bootloader) {
if (STREQ(def-os.type, hvm))
hvm = 1;
@@ -5453,9 +5463,6 @@ xenDaemonFormatSxpr(virConnectPtr conn,
virBufferAddLit(buf, (serial none));
}
-if (def-localtime)
-virBufferAddLit(buf, (localtime 1));
-
if (def-sounds) {
virBufferAddLit(buf, (soundhw ');
if (xenDaemonFormatSxprSound(conn, def, buf) 0)
diff --git a/src/xm_internal.c b/src/xm_internal.c
--- a/src/xm_internal.c
+++ b/src/xm_internal.c
@@ -818,10 +818,11 @@ xenXMDomainConfigParse(virConnectPtr con
goto cleanup;
else if (val)
def-features |= (1 VIR_DOMAIN_FEATURE_APIC);
-
-if (xenXMConfigGetBool(conn, conf, localtime, def-localtime, 0)
0)
-goto cleanup;
-}
+}
+
+if (xenXMConfigGetBool(conn, conf, localtime, def-localtime, 0) 0)
+goto cleanup;
+
if (xenXMConfigCopyStringOpt(conn, conf, device_model, def-emulator)
0)
goto cleanup;
@@ -1972,6 +1973,9 @@ virConfPtr xenXMDomainConfigFormat(virCo
if (xenXMConfigSetInt(conf, vcpus, def-vcpus) 0)
goto no_memory;
+if (def-localtime xenXMConfigSetInt(conf, localtime, 1) 0)
+goto no_memory;
+
if (def-cpumask
!(cpus = virDomainCpuSetFormat(conn, def-cpumask, def-cpumasklen))
0)
goto cleanup;
@@ -2034,9 +2038,6 @@ virConfPtr xenXMDomainConfigFormat(virCo
(1 VIR_DOMAIN_FEATURE_APIC)) ? 1 : 0) 0)
goto no_memory;
-
-if (xenXMConfigSetInt(conf, localtime, def-localtime ? 1 : 0) 0)
-goto no_memory;
if (priv-xendConfigVersion == 1) {
for (i = 0 ; i def-ndisks ; i++) {
diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-localtime2.sexpr
b/tests/sexpr2xmldata/sexpr2xml-fv-localtime2.sexpr
new file mode 100644
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-fv-localtime2.sexpr
@@ -0,0 +1,1 @@
+(domain (domid 3)(name 'fvtest')(memory 400)(maxmem 400)(vcpus 1)(uuid
'b5d70dd275cdaca517769660b059d8bc')(on_poweroff 'destroy')(on_reboot
'restart')(on_crash 'restart')(localtime 1)(image (hvm (kernel
'/usr/lib/xen/boot/hvmloader')(device_model '/usr/lib64/xen/bin/qemu-dm')(boot
c)(cdrom '/root/boot.iso')(acpi 1)(vnc 1)(keymap ja)))(device (vbd (dev
'ioemu:hda')(uname 'file:/root/foo.img')(mode 'w')))(device (vif (mac
'00:16:3e:1b:b1:47')(bridge 'xenbr0')(script 'vif-bridge')(type ioemu
diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-localtime2.xml
b/tests/sexpr2xmldata/sexpr2xml-fv-localtime2.xml
new file mode 100644
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-fv-localtime2.xml
@@ -0,0 +1,41 @@
+domain type='xen' id='3'
+ namefvtest/name
+ uuidb5d70dd2-75cd-aca5-1776-9660b059d8bc/uuid
+ memory409600/memory
+ currentMemory409600/currentMemory
+ vcpu1/vcpu
+ os
+typehvm/type
+loader/usr/lib/xen/boot/hvmloader/loader
+boot dev='hd'/
+ /os
+ features
+acpi/
+ /features
+ clock offset='localtime'/
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashrestart/on_crash
+ devices
+emulator/usr/lib64/xen/bin/qemu-dm/emulator
+disk type='file' device='disk'
+ driver name='file'/
+ source file='/root/foo.img'/
+ target dev='hda' bus='ide'/
+/disk
+disk type='file' device='cdrom'
+ driver name='file'/
+ source file='/root/boot.iso'/
+ target dev='hdc' bus='ide'/
+ readonly/
+/disk
+interface type='bridge'
+ mac address='00:16:3e:1b:b1:47'/
+ source
[libvirt] [PATCH] Fix another printf(%s, NULL) case
# HG changeset patch
# User john.le...@sun.com
# Date 1233199841 28800
# Node ID 3f2af372963ddbae509bd0d1b2e7bdf3ebb4f217
# Parent 76d670ab6ea5476be96441ea88af94644c985201
Fix another printf(%s, NULL) case
Signed-off-by: John Levon john.le...@sun.com
diff --git a/src/libvirt.c b/src/libvirt.c
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -6922,7 +6922,7 @@ int
int
virNodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int flags)
{
-DEBUG(conn=%p, cap=%s, flags=%d, conn, cap, flags);
+DEBUG(conn=%p, cap=%s, flags=%d, conn, NULLSTR(cap), flags);
virResetLastError();
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] A small example program
The following patches provide a small example program that illustrates how to connect to libvirt, make a few API calls and disconnect. The first patch is my first cut at it with feedback from Rich Jones. The second patch implements feedback from Jim Meyering. The third patch is completely new and implements (I think) integration with the libvirt build tools, so I would appreciate careful examination of what I've done there. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] Add autoconf/automake support
Added examples/hellolibvirt to the parent autoconf/automake files and created Makefile.am --- Makefile.am |2 +- configure.in |3 ++- examples/hellolibvirt/Makefile.am |5 + 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 examples/hellolibvirt/Makefile.am diff --git a/Makefile.am b/Makefile.am index d4e42f1..928a93c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4,7 +4,7 @@ LCOV = lcov GENHTML = genhtml SUBDIRS = gnulib/lib include src qemud proxy docs gnulib/tests \ - python tests po examples/domain-events/events-c + python tests po examples/domain-events/events-c examples/hellolibvirt ACLOCAL_AMFLAGS = -I m4 -I gnulib/m4 diff --git a/configure.in b/configure.in index 493ea28..3db0976 100644 --- a/configure.in +++ b/configure.in @@ -1285,7 +1285,8 @@ AC_OUTPUT(Makefile src/Makefile include/Makefile docs/Makefile \ tests/xmconfigdata/Makefile \ tests/xencapsdata/Makefile \ tests/confdata/Makefile \ - examples/domain-events/events-c/Makefile) + examples/domain-events/events-c/Makefile \ + examples/hellolibvirt/Makefile) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Configuration summary]) diff --git a/examples/hellolibvirt/Makefile.am b/examples/hellolibvirt/Makefile.am new file mode 100644 index 000..6ef2cc8 --- /dev/null +++ b/examples/hellolibvirt/Makefile.am @@ -0,0 +1,5 @@ +INCLUDES = -...@top_srcdir@/include +noinst_PROGRAMS = hellolibvirt +hellolibvirt_CFLAGS = $(WARN_CFLAGS) +hellolibvirt_SOURCES = hellolibvirt.c +hellolibvirt_LDADD = @top_builddir@/src/libvirt.la -- 1.6.0.6 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Changes per Jim Meyering
Handle failure of virConnectNumOfDomains and virConnectNumOfDefinedDomains
Changed malloc to use size of variable, not type
Removed unneeded NULL check before free
Removed unneeded initialization of several variables
Changed assignment to use the ternary operator
---
examples/hellolibvirt/hellolibvirt.c | 28 ++--
1 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/examples/hellolibvirt/hellolibvirt.c
b/examples/hellolibvirt/hellolibvirt.c
index 22d3309..cc1af0f 100644
--- a/examples/hellolibvirt/hellolibvirt.c
+++ b/examples/hellolibvirt/hellolibvirt.c
@@ -1,6 +1,8 @@
/* This file contains trivial example code to connect to the running
* hypervisor and gather a few bits of information. */
+#include config.h
+
#include stdio.h
#include stdlib.h
#include libvirt/libvirt.h
@@ -52,12 +54,23 @@ showDomains(virConnectPtr conn)
char **nameList = NULL;
numActiveDomains = virConnectNumOfDomains(conn);
+if (-1 == numActiveDomains) {
+ret = 1;
+printf(Failed to get number of active domains\n);
+goto out;
+}
+
numInactiveDomains = virConnectNumOfDefinedDomains(conn);
+if (-1 == numInactiveDomains) {
+ret = 1;
+printf(Failed to get number of inactive domains\n);
+goto out;
+}
printf(There are %d active and %d inactive domains\n,
numActiveDomains, numInactiveDomains);
-nameList = malloc(sizeof(char *) * (unsigned int)numInactiveDomains);
+nameList = malloc(sizeof(*nameList) * numInactiveDomains);
if (NULL == nameList) {
ret = 1;
@@ -88,10 +101,7 @@ showDomains(virConnectPtr conn)
}
out:
-if (NULL != nameList) {
-free(nameList);
-}
-
+free(nameList);
return ret;
}
@@ -100,14 +110,12 @@ int
main(int argc, char *argv[])
{
int ret = 0;
-virConnectPtr conn = NULL;
-char *uri = NULL;
+virConnectPtr conn;
+char *uri;
printf(Attempting to connect to hypervisor\n);
-if (argc 0) {
-uri = argv[1];
-}
+uri = (argc 0 ? argv[1] : NULL);
/* virConnectOpenAuth is called here with all default parameters,
* except, possibly, the URI of the hypervisor. */
--
1.6.0.6
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] Add a trivial example program
This example code illustrates connecting to the hypervisor and making some
simple API calls.
Added a little code to let the user specify the URI of the hypervisor on the
command line, per the suggestion of Rich Jones.
---
examples/hellolibvirt/hellolibvirt.c | 151 ++
1 files changed, 151 insertions(+), 0 deletions(-)
create mode 100644 examples/hellolibvirt/hellolibvirt.c
diff --git a/examples/hellolibvirt/hellolibvirt.c
b/examples/hellolibvirt/hellolibvirt.c
new file mode 100644
index 000..22d3309
--- /dev/null
+++ b/examples/hellolibvirt/hellolibvirt.c
@@ -0,0 +1,151 @@
+/* This file contains trivial example code to connect to the running
+ * hypervisor and gather a few bits of information. */
+
+#include stdio.h
+#include stdlib.h
+#include libvirt/libvirt.h
+
+static int
+showHypervisorInfo(virConnectPtr conn)
+{
+int ret = 0;
+unsigned long hvVer, major, minor, release;
+const char *hvType;
+
+/* virConnectGetType returns a pointer to a static string, so no
+ * allocation or freeing is necessary; it is possible for the call
+ * to fail if, for example, there is no connection to a
+ * hypervisor, so check what it returns. */
+hvType = virConnectGetType(conn);
+if (NULL == hvType) {
+ret = 1;
+printf(Failed to get hypervisor type\n);
+goto out;
+}
+
+if (0 != virConnectGetVersion(conn, hvVer)) {
+ret = 1;
+printf(Failed to get hypervisor version\n);
+goto out;
+}
+
+major = hvVer / 100;
+hvVer %= 100;
+minor = hvVer / 1000;
+release = hvVer % 1000;
+
+printf(Hypervisor: \%s\ version: %lu.%lu.%lu\n,
+ hvType,
+ major,
+ minor,
+ release);
+
+out:
+return ret;
+}
+
+
+static int
+showDomains(virConnectPtr conn)
+{
+int ret = 0, i, numNames, numInactiveDomains, numActiveDomains;
+char **nameList = NULL;
+
+numActiveDomains = virConnectNumOfDomains(conn);
+numInactiveDomains = virConnectNumOfDefinedDomains(conn);
+
+printf(There are %d active and %d inactive domains\n,
+ numActiveDomains, numInactiveDomains);
+
+nameList = malloc(sizeof(char *) * (unsigned int)numInactiveDomains);
+
+if (NULL == nameList) {
+ret = 1;
+printf(Could not allocate memory for list of inactive domains\n);
+goto out;
+}
+
+numNames = virConnectListDefinedDomains(conn,
+nameList,
+numInactiveDomains);
+
+if (-1 == numNames) {
+ret = 1;
+printf(Could not get list of defined domains from hypervisor\n);
+goto out;
+}
+
+if (numNames 0) {
+printf(Inactive domains:\n);
+}
+
+for (i = 0 ; i numNames ; i++) {
+printf( %s\n, *(nameList + i));
+/* The API documentation doesn't say so, but the names
+ * returned by virConnectListDefinedDomains are strdup'd and
+ * must be freed here. */
+free(*(nameList + i));
+}
+
+out:
+if (NULL != nameList) {
+free(nameList);
+}
+
+return ret;
+}
+
+
+int
+main(int argc, char *argv[])
+{
+int ret = 0;
+virConnectPtr conn = NULL;
+char *uri = NULL;
+
+printf(Attempting to connect to hypervisor\n);
+
+if (argc 0) {
+uri = argv[1];
+}
+
+/* virConnectOpenAuth is called here with all default parameters,
+ * except, possibly, the URI of the hypervisor. */
+conn = virConnectOpenAuth(uri, virConnectAuthPtrDefault, 0);
+
+if (NULL == conn) {
+ret = 1;
+printf(No connection to hypervisor\n);
+goto out;
+}
+
+uri = virConnectGetURI(conn);
+if (NULL == uri) {
+ret = 1;
+printf(Failed to get URI for hypervisor connection\n);
+goto disconnect;
+}
+
+printf(Connected to hypervisor at \%s\\n, uri);
+free(uri);
+
+if (0 != showHypervisorInfo(conn)) {
+ret = 1;
+goto disconnect;
+}
+
+if (0 != showDomains(conn)) {
+ret = 1;
+goto disconnect;
+}
+
+disconnect:
+if (0 != virConnectClose(conn)) {
+printf(Failed to disconnect from hypervisor\n);
+} else {
+printf(Disconnected from hypervisor\n);
+}
+
+out:
+return ret;
+}
--
1.6.0.6
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix xend XML generation when CPU pinning is used
# HG changeset patch
# User john.le...@sun.com
# Date 1233203295 28800
# Node ID 9aac8028d5f266023c85338afe71ecabe26079f7
# Parent add5d46423d8cc24ac922373ba5cd1b3ea2e6f9f
Fix xend XML generation when CPU pinning is used
Signed-off-by: John Levon john.le...@sun.com
diff --git a/src/xend_internal.c b/src/xend_internal.c
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -2227,11 +2227,21 @@ xenDaemonParseSxpr(virConnectPtr conn,
def-maxmem = def-memory;
if (cpus != NULL) {
+def-cpumasklen = VIR_DOMAIN_CPUMASK_LEN;
+if (VIR_ALLOC_N(def-cpumask, def-cpumasklen) 0) {
+virXendError(conn, VIR_ERR_NO_MEMORY, NULL);
+goto error;
+}
+
if (virDomainCpuSetParse(conn, cpus,
0, def-cpumask,
- def-cpumasklen) 0)
+ def-cpumasklen) 0) {
+virXendError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(invalid CPU mask %s), cpus);
goto error;
-}
+}
+}
+
def-vcpus = sexpr_int(root, domain/vcpus);
tmp = sexpr_node(root, domain/on_poweroff);
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Question about libvirt-qpid
2 questions. 1)Would you explain the use case for libvirt-qpid? Which case AMDQ messaging is good for. If I am missing some document, Please point me. By the way I am reading through following document. http://jira.amqp.org/confluence/download/attachments/2523279/amqp-model-draft101.pdf I guess this is fit for huge data case? 2)Where is the document about set up libvirt-qpid? There is not in package document. Only in mailing list, it should goes some document. https://www.redhat.com/archives/libvir-list/2008-September/msg00388.html Thanks Atsushi SAKAI -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [Patch][RFC] Fine grained access control in libvirt by rbac (0/3)
Hi, it seems that we share the interest. Konrad Eriksson1 wrote: Yes, we're looking into adding similar form of access control in libvirt. The approach we're looking at is to inject AC as a module that intercepts calls from the libvirt core (libvirt.c) to the drivers. Reason: * AC module can be loaded/unloaded on the fly without need to recompile (can support several different AC-modules and load the appropriate one during connect). * Making use of (semi-stable) API between libvirt core and drivers (also hypervisor/storage/network driver independent) * Being in the call-path also enables AC-module to alter return values (such as filtering lists of VMs/NETs/Storages based on access rights) * Minimal code changes in existing libvirt code (basically a one-liner in libvirt.c to inject AC) Sounds reasonable. Injecting AC without embedding hooks at API entries is pretty cool. Do you have any implementation? What still is an issue is how to correctly get the identity of the user, especially over remote connection. I guess you have the same problem? (you're only allowing local usage for now). Yes, our AC takes effect only when it is invoked locally because uid is unavailable at a remote site. The best way would be to link some user-auth data with the virConnectPtr, but becomes a bit trickier when authentication is done prior (like in remote case) to virConnectOpen. I agree in that some user-auth mechanisms are necessary, but now I concentrate on AC-module itself assuming that user-auth is finished. Daniel gave a concrete idea on user authentication and I find it fine. You implemented your own RBAC language to describe the AC-policies. Have you looked at possibility to link with already existing RBAC mechanisms for Linux (like SELinux or maybe simpler AC-libs)? I have some coarse ideas where policies should be given as a SELinux's policy. It extends libvirt, like sVirt does, so that it calls SELinux API to make an authenticated user move to a qualified role (domain). Current sVirt is something different from AC itself though it employs SELinux. sVirt constrains qemu processes to use just allowed objects, while AC-module is aimed to constrain users to call just allowed sub commands on allowd VMs. From security point of view, sVirt together with SElinux constraining libvirt is secure enough to me. So, I don't think it is mandatory that AC be realized by using SELinux. The main goal of AC in this context is to prevent differently authorized user from accidentally invoking unallowed operations. This seems a bit different from keeping the whole server untainted. So, proprietary implementations don't likely cause a serious security threat. Of cause, I understand that SELinux appears an attractive option. Regards, Hiroya Freundliche Grusse / Best regards *Konrad Eriksson* Trusted Computing / Security Assurance Email: _...@zurich.ibm.com_ mailto:k...@zurich.ibm.com Phone: +41 (0)44 724 84 28* IBM Zurich Research Laboratory*_ __www.zurich.ibm.com_ http://www.zurich.ibm.com/ Saeumerstrasse 4 8803 Rueschlikon Switzerland From: Syunsuke HAYASHI syuns...@jp.fujitsu.com To: libvir-list@redhat.com Cc: Konrad Eriksson1 k...@zurich.ibm.com, berra...@redhat.com, Atsushi SAKAI sak...@jp.fujitsu.com, INAKOSHI Hiroya inakoshi.hir...@jp.fujitsu.com Date: 01/26/2009 11:25 AM Subject: [Patch][RFC] Fine grained access control in libvirt by rbac (0/3) The series of patches introduces a fine grained access control to libvirt. They enable libvirt to enforce users what operations to invoke in role-based way. Our team found that Konrad and Daniel have similar interest to ours. Comments and suggestions are very welcome. Patches: - Embedding hooks in libvirt (1/3) - Access control library (2/3) - Example policy files (3/3) -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [Patch][RFC] Fine grained access control in libvirt by rbac (0/3)
Hi, thanks for your analysis. Daniel P. Berrange wrote: There are lots of scenarios to consider - Direct local API usage. You have the PID, UID GID of the process - Local usage via libvirtd + UNIX sockets. You can get the PID, UID GID of the client end using the SCM_CREDENTIALS message. (see man 7 unix) - Remote usage via libvirtd + TCP sockets. Depending on the security encryption settings you may have a SASL username, or a x509 certificate CNAME, or both, or neither. - Local usage via libvirtd + UNIX sockets + libvirt-qpid. The PID, UID GID of the client end aren't particularly usage, since libvirt-qpid is just a demon running as root, which accepts calls on behalf of many remote apps. Does QPid provide any identifying info about the entity which put the message on the queue. I'm not familier with Qpid. Could you explain its benefit or point me some documents about it? And how can I use it on libvirt? - Remote usage with IPSec ? I personally don't like IPSec because it's too rigid. And I don't know whether it is common. X509, SASL, or password authentication through secure connection seems simple and good enough. So there are multiple identifying credentials, in multiple formats, and need some way to associate this information with a connection. Applying RBAC to local (non-Daemon) API usage has clear limitations - if the user running virsh (or equiv) has direct access to the system, then they could trivially just replace the real virsh with their own virsh without RBAC. So RBAC usage in the non-Daemon context is only useful if the user does not have direct access to the ssytem (eg, virsh is being invoked on their behalf by another tool, or a constrained environment where its guarenteed they can't provide their own libraries/binaries. That's true but, as I mentioned in the other e-mail, I'm now concentrating on AC itself assuming that user-auth has been established. I don't think user authentication and access control should be tied up with each other. I mean, it's nice if AC-module can always use uid as a part of the key for consulting policies, not depending on whether libvirt is running on a local or remote server. The best way would be to link some user-auth data with the virConnectPtr, but becomes a bit trickier when authentication is done prior (like in remote case) to virConnectOpen. The key question is do we need to pass the client/callers identity at the time we create the connection object, or is it sufficient to provide it after the fact with a call like virConnectAddSecParam(VIR_SECPARAM_UNIX_UID, getuid()); virConnectAddSecParam(VIR_SECPARAM_UNIX_GID, getgid()); virConnectAddSecParam(VIR_SECPARAM_USERNAME, saslUsername); virConnectAddSecParam(VIR_SECPARAM_X509_CNAME, saslUsername); A simple question. How do you identify what user the remote libvirt switches to? Do you look up some directory services? Hiroya Or do we need to provide this info via some form of callback mechanism, perhaps via the exiting virConnectCredential struct, which is curently not used on the server end of remote connections. Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
