[libvirt] [PATCH] Use SED variable for `sed` binary in src/Makefile

2010-09-09 Thread Mitchell Hashimoto
Hi,

I've been trying to get libvirt (client) to cleanly/easily compile on
BSD-based systems (in this case OS X). ./configure runs fine but the
make caused an error with `sed` since BSD sed was reporting some sort of
regex error. I realized that the SED variable was populated with gsed
which worked properly. This made the make continue (failed again later, will
address that when I can).

This is my first contribution to a C-based project and I don't have much
experience with autotools other than using them as a consumer. Let me know
if anything can be improved.

Thank you,
Mitchell


0001-Use-SED-variable-for-sed-binary-in-src-Makefile.patch
Description: Binary data
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Use SED variable for `sed` binary in src/Makefile

2010-09-09 Thread Mitchell Hashimoto
Hm, the patch attached as binary data. Here is the attached patch as text.
Apologies about that:

---
 src/Makefile.am |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index b321657..118c329 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1029,14 +1029,14 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES)
 libvirt.def: libvirt.syms
$(AM_V_GEN)rm -f -- $...@-tmp $@ ; \
printf 'EXPORTS\n'  $...@-tmp  \
-   sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
\t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
+   $(SED) -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
\t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
chmod a-w $...@-tmp  \
mv $...@-tmp libvirt.def

 libvirt_qemu.def: $(srcdir)/libvirt_qemu.syms
$(AM_V_GEN)rm -f -- $...@-tmp $@ ; \
printf 'EXPORTS\n'  $...@-tmp  \
-   sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
\t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
+   $(SED) -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
\t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
chmod a-w $...@-tmp  \
mv $...@-tmp libvirt_qemu.def

-- 
1.7.2.2

On Wed, Sep 8, 2010 at 11:39 PM, Mitchell Hashimoto 
mitchell.hashim...@gmail.com wrote:

 Hi,

 I've been trying to get libvirt (client) to cleanly/easily compile on
 BSD-based systems (in this case OS X). ./configure runs fine but the
 make caused an error with `sed` since BSD sed was reporting some sort of
 regex error. I realized that the SED variable was populated with gsed
 which worked properly. This made the make continue (failed again later, will
 address that when I can).

 This is my first contribution to a C-based project and I don't have much
 experience with autotools other than using them as a consumer. Let me know
 if anything can be improved.

 Thank you,
 Mitchell

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Question, how to use virDomainQemuMonitorCommand()

2010-09-09 Thread Lai Jiangshan
On 09/07/2010 09:22 PM, Chris Lalancette wrote:
 On 09/07/10 - 04:08:13PM, Lai Jiangshan wrote:
 Hi, Chris,

 I saw virDomainQemuMonitorCommand() in libvirt-qemu.c,
 I think it will help me to send arbitrary qemu-monitor command to
 qemu via libvirtd.

 But how can I use virDomainQemuMonitorCommand()?
 Can I use it by just using current tools(virsh or other) without writing any 
 code?
 
 Unfortunately, no.  There is a bug in the current virsh command that prevents
 it from properly parsing the command-lines necessary to send monitor commands
 to the qemu monitor.  Until we fix that bug, we won't push the support into
 virsh.
 

Thanks,

We need this feature, could you tell me the detail of the bug,
we will try to fix it or do assists.

Thanks a lot.
Lai.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] docs: add the app dev guide

2010-09-09 Thread Justin Clift
   Added a workable initial page for the libvirt Application
   Development Guide, giving the online viewable options +
   the downloadable pdf.

   Added a link to the PDF to the main Downloads page, plus
   neatened the html tags throughout the page as they
   were a bit of a mess.
---
 docs/devguide.html.in  |   44 ++
 docs/downloads.html.in |  150 +++-
 docs/sitemap.html.in   |4 +
 3 files changed, 146 insertions(+), 52 deletions(-)
 create mode 100644 docs/devguide.html.in

diff --git a/docs/devguide.html.in b/docs/devguide.html.in
new file mode 100644
index 000..60feee0
--- /dev/null
+++ b/docs/devguide.html.in
@@ -0,0 +1,44 @@
+?xml version=1.0?
+html
+  body
+h1libvirt Application Development Guide/h1
+
+p
+  This is both a guide to developing with libvirt, and a useful
+  reference document.  It is a work in progress, contributed to by the
+  members of the libvirt team and being authored by a professional
+  author.
+/p
+
+p
+  Contributors to this are bVERY/b welcome, so if you'd like to
+  get your name in this and demonstrate your virtualisation prowess,
+  contributing solidly to the content here will do it. :)
+/p
+
+h2Browsable online/h2
+
+ul
+  lia href=http://libvirt.org/guide/html/;HTML format using multiple 
pages/a/li
+  lia href=http://libvirt.org/guide/html-single/;HTML format using 
one big page/a/li
+  lia 
href=http://libvirt.org/guide/pdf/Application_Development_Guide.pdf;PDF 
format/a/li
+/ul
+
+h2GIT source repository/h2
+
+p
+  The source is in a git repository:
+/p
+
+pre
+  git clone git://libvirt.org/libvirt-appdev-guide.git/pre
+
+p
+  Browsable at:
+/p
+
+pre
+  a 
href=http://libvirt.org/git/?p=libvirt-appdev-guide.git;a=summary;http://libvirt.org/git/?p=libvirt-appdev-guide.git;a=summary/a/pre
+
+  /body
+/html
diff --git a/docs/downloads.html.in b/docs/downloads.html.in
index 6872272..01f2e4c 100644
--- a/docs/downloads.html.in
+++ b/docs/downloads.html.in
@@ -1,7 +1,7 @@
 ?xml version=1.0?
 html
   body
-h1 Downloads/h1
+h1Downloads/h1
 
 h2Official Releases/h2
 
@@ -27,62 +27,108 @@
   lia 
href=http://libvirt.org/sources/libvirt-git-snapshot.tar.gz;libvirt.org HTTP 
server/a/li
 /ul
 
-  h2GIT source repository/h2
-  p Libvirt code source is now maintained in a a
-  href=http://git-scm.com/;git/a repository available on
-  a href=http://libvirt.org/git/;libvirt.org/a:
+h2GIT source repository/h2
+
+p
+  Libvirt code source is now maintained in a a 
href=http://git-scm.com/;git/a
+  repository available on a 
href=http://libvirt.org/git/;libvirt.org/a:
+/p
+
+pre
+  git clone git://libvirt.org/libvirt.git/pre
+
+p
+  It can also be browsed at:
+/p
+
+pre
+  a 
href=http://libvirt.org/git/?p=libvirt.git;a=summary;http://libvirt.org/git/?p=libvirt.git;a=summary/a/pre
+
+br /
+
+h1libvirt Application Development Guide/h1
+
+p
+  This is both a guide to developing with libvirt, and a useful
+  reference document.  It is a work in progress, contributed to by the
+  members of the libvirt team and being authored by a professional
+  author.
+/p
+
+p
+  Contributors to this are bVERY/b welcome, so if you'd like to
+  get your name in this and demonstrate your virtualisation prowess,
+  contributing solidly to the content here will do it. :)
+/p
+
+h2Downloadable PDF/h2
+
+p
+  PDF download is available here:
+/p
+
+ul
+  lia 
href=http://libvirt.org/guide/pdf/Application_Development_Guide.pdf;libvirt 
App Dev Guide/a (PDF)/li
+/ul
+
+h2GIT source repository/h2
+
+p
+  The source is also in a git repository:
+/p
+
+pre
+  git clone git://libvirt.org/libvirt-appdev-guide.git/pre
+
+p
+  Browsable at:
+/p
+
+pre
+  a 
href=http://libvirt.org/git/?p=libvirt-appdev-guide.git;a=summary;http://libvirt.org/git/?p=libvirt-appdev-guide.git;a=summary/a/pre
+
+br /
+
+h1libvirt Installation/h1
+
+h2a name=Compilatio id=CompilatioCompilation/a/h2
+
+p
+  libvirt uses the standard configure/make/install steps:
+/p
+
+pre
+  gunzip -c libvirt-xxx.tar.gz | tar xvf -
+  cd libvirt-
+  ./configure --help/pre
+
+p
+  To see the options, then the compilation/installation proper:
+/p
+
+pre
+  ./configure [possible options]
+  make
+  make install/pre
+
+p
+  At that point you may have to rerun ldconfig or a similar utility to
+  update your list of installed shared libs.
 /p
-pre
 
-  git clone git://libvirt.org/libvirt.git
-/pre
-p
-  It can also be browsed at
+h2Building from a source code checkout/h2
+
+p
+  The libvirt build process uses GNU autotools, so after obtaining a
+  

Re: [libvirt] Question, how to use virDomainQemuMonitorCommand()

2010-09-09 Thread Chris Lalancette
On 09/09/10 - 04:52:25PM, Lai Jiangshan wrote:
 On 09/07/2010 09:22 PM, Chris Lalancette wrote:
  On 09/07/10 - 04:08:13PM, Lai Jiangshan wrote:
  Hi, Chris,
 
  I saw virDomainQemuMonitorCommand() in libvirt-qemu.c,
  I think it will help me to send arbitrary qemu-monitor command to
  qemu via libvirtd.
 
  But how can I use virDomainQemuMonitorCommand()?
  Can I use it by just using current tools(virsh or other) without writing 
  any code?
  
  Unfortunately, no.  There is a bug in the current virsh command that 
  prevents
  it from properly parsing the command-lines necessary to send monitor 
  commands
  to the qemu monitor.  Until we fix that bug, we won't push the support into
  virsh.
  
 
 Thanks,
 
 We need this feature, could you tell me the detail of the bug,
 we will try to fix it or do assists.

I've outlined it before on this list, but the gist of it is that the way that
virsh parses command-line arguments loses the formatting.  Thus, if you were
enter a command like:

# virsh qemu-monitor-command f13guest info cpus

Then virsh main() will get 4 arguments:

argv[0] = virsh;
argv[1] = qemu-monitor-command;
argv[2] = f13guest;
argv[3] = info cpus;

So far, all is good.  However, during the parsing of these command-line
arguments, virsh takes all of these arguments and smashes them back together
as a single string:

command = virsh qemu-monitor-command f13guest info cpus;

And then it reparses the whole thing.  Notice that we've lost the quoting,
though, so now it's an invalid command.

The problem is further complicated by some of the other features of virsh,
including the support for separating multiple commands with semicolons.  For
example, the following is a legal command:

# virsh 'define D.xml; dumpxml D'

In any case, the answer is probably to re-write the command-line parsing of
virsh to not lose quoting.  I have not had time to do this, so if you have
the time to look at it and make it work, patches are definitely appreciated!

-- 
Chris Lalancette

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] Question, how to use virDomainQemuMonitorCommand()

2010-09-09 Thread Daniel P. Berrange
On Thu, Sep 09, 2010 at 08:52:13AM -0400, Chris Lalancette wrote:
 On 09/09/10 - 04:52:25PM, Lai Jiangshan wrote:
  On 09/07/2010 09:22 PM, Chris Lalancette wrote:
   On 09/07/10 - 04:08:13PM, Lai Jiangshan wrote:
   Hi, Chris,
  
   I saw virDomainQemuMonitorCommand() in libvirt-qemu.c,
   I think it will help me to send arbitrary qemu-monitor command to
   qemu via libvirtd.
  
   But how can I use virDomainQemuMonitorCommand()?
   Can I use it by just using current tools(virsh or other) without writing 
   any code?
   
   Unfortunately, no.  There is a bug in the current virsh command that 
   prevents
   it from properly parsing the command-lines necessary to send monitor 
   commands
   to the qemu monitor.  Until we fix that bug, we won't push the support 
   into
   virsh.
   
  
  Thanks,
  
  We need this feature, could you tell me the detail of the bug,
  we will try to fix it or do assists.
 
 I've outlined it before on this list, but the gist of it is that the way that
 virsh parses command-line arguments loses the formatting.  Thus, if you were
 enter a command like:
 
 # virsh qemu-monitor-command f13guest info cpus
 
 Then virsh main() will get 4 arguments:
 
 argv[0] = virsh;
 argv[1] = qemu-monitor-command;
 argv[2] = f13guest;
 argv[3] = info cpus;
 
 So far, all is good.  However, during the parsing of these command-line
 arguments, virsh takes all of these arguments and smashes them back together
 as a single string:
 
 command = virsh qemu-monitor-command f13guest info cpus;
 
 And then it reparses the whole thing.  Notice that we've lost the quoting,
 though, so now it's an invalid command.
 
 The problem is further complicated by some of the other features of virsh,
 including the support for separating multiple commands with semicolons.  For
 example, the following is a legal command:
 
 # virsh 'define D.xml; dumpxml D'
 
 In any case, the answer is probably to re-write the command-line parsing of
 virsh to not lose quoting.  I have not had time to do this, so if you have
 the time to look at it and make it work, patches are definitely appreciated!

While re-writing the command line mashing to not loose quotes would be
nice, the more obvious fix is to not mash all the args back into a
string at all. The virsh command ultimately wants char **argv, and we 
already have char **argv. So doing a char ** - char * - char **
conversion is just insanity.

Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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 v2] [TCK] [REPOST] nwfilter: apply filters and check firewall rules

2010-09-09 Thread Daniel P. Berrange
On Tue, Jul 13, 2010 at 09:41:42PM -0400, Stefan Berger wrote:
 Any comments regarding this patch?
 
 V2:
  - Following Daniel Berrange's suggestions
- if LIBVIRT_TCK_CONFIG is set, read the last occurrence of ^uri/s= and 
 assign the value to LIBVIRT_URI
- check that LIBVIRT_URI is set to qemu:///system, otherwise skip test
- if allowed, remove all VMs and nwfilters starting with tck-
- rename all VMs and nwfilters created by this test program to start with 
 'tck-'
  - other:
- terminate if sourcing the test-lib.sh from libvirt's tests dir is 
 missing (would need to be copied)
- redirect stderr to stdout whereever output is read into a variable

Sorry I completely forgot about this. I've now tested it and
with a couple of minor fixes it works fine. I just had to
tweak the way it extracted the libvirt uri from the tck config,
and remove the hardcoded paths to libvirtd/virsh that expect
a source tree. The TCK is designed to run an installed system.
If it is desired to run against an install tree, those can just
be added to $PATH  libvirtd started ahead of time.

I've committed the patch  pushed it to GIT

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] Use SED variable for `sed` binary in src/Makefile

2010-09-09 Thread Richard W.M. Jones
On Wed, Sep 08, 2010 at 11:41:52PM -0700, Mitchell Hashimoto wrote:
 Hm, the patch attached as binary data. Here is the attached patch as text.
 Apologies about that:
 
 ---
  src/Makefile.am |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/Makefile.am b/src/Makefile.am
 index b321657..118c329 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -1029,14 +1029,14 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES)
  libvirt.def: libvirt.syms
 $(AM_V_GEN)rm -f -- $...@-tmp $@ ; \
 printf 'EXPORTS\n'  $...@-tmp  \
 -   sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
 \t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
 +   $(SED) -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
 \t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
 chmod a-w $...@-tmp  \
 mv $...@-tmp libvirt.def
 
  libvirt_qemu.def: $(srcdir)/libvirt_qemu.syms
 $(AM_V_GEN)rm -f -- $...@-tmp $@ ; \
 printf 'EXPORTS\n'  $...@-tmp  \
 -   sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
 \t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
 +   $(SED) -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
 \t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
 chmod a-w $...@-tmp  \
 mv $...@-tmp libvirt_qemu.def

I think you also need to add AC_PROG_SED to configure.ac.  At the
moment it is being defined, but only incidentally because of some
other macro.

There are also uses of sed, at least one in configure.ac itself ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.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 0/3] test cases for spoofing prevention

2010-09-09 Thread Daniel P. Berrange
On Wed, Jun 16, 2010 at 04:08:00PM +0200, gsten...@linux.vnet.ibm.com wrote:
 The following patches add a set of test cases to verify that several spoofing 
 attacks are prevented by the nwfilter subsystem.
 
 In order to have a well defined test machine a virtual disk is installed from 
 scratch over the network.
 I am currently trying to find a suitable location for the kickstart file.

Do you have the suitable 'ks.cfg' you used with these test scripts ? The
test files look good to me and I'm going to commit them all now. We just
need the ks.cfg so we can make it work - I'll make it pull it off a floppy
disk image

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] SVG sources for the libvirt wiki images is online

2010-09-09 Thread Justin Clift

Hi all,

The SVG sources for the images used in the libvirt wiki docs are online:

  http://wiki.libvirt.org/page/SVGImages

That's a browsable list, still fairly disorganised (!), but contains a 
visual list of the images on the wiki so far, with the link to their SVG 
source.


They're all licensed under the Creative Commons Attribution–Share Alike 
3.0 Unported license (CC-BY-SA):


  http://creativecommons.org/licenses/by-sa/3.0/

Feel welcome to use them in your wiki's, improve them, and so on.

Updates, ideas, and contributions welcome too of course. :)

Regards and best wishes,

Justin Clift

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 0/3] test cases for spoofing prevention

2010-09-09 Thread Gerhard Stenzel
On Thu, 2010-09-09 at 14:48 +0100, Daniel P. Berrange wrote:
 On Wed, Jun 16, 2010 at 04:08:00PM +0200, gsten...@linux.vnet.ibm.com wrote:
  The following patches add a set of test cases to verify that several 
  spoofing attacks are prevented by the nwfilter subsystem.
  
  In order to have a well defined test machine a virtual disk is installed 
  from scratch over the network.
  I am currently trying to find a suitable location for the kickstart file.
 
 Do you have the suitable 'ks.cfg' you used with these test scripts ? The
 test files look good to me and I'm going to commit them all now. We just
 need the ks.cfg so we can make it work - I'll make it pull it off a floppy
 disk image
 
 Regards,
 Daniel

Here is the one I used. I could update it to a newer fedora version, if
necessary:

#version=F12
install
text
url
--url=http://ftp-stud.hs-esslingen.de/Mirrors/fedora.redhat.com/linux/releases/12/Fedora/i386/os/
lang en_US.UTF-8
keyboard de-latin1-nodeadkeys
network --device eth0 --bootproto dhcp
rootpw  --iscrypted $6$AHEMvpa2rx3n/DON
$toWNA/ainpreIRC1g2L9yuil7bS.2hIf8DomTluFGulQtN3KstPeVrmwFMhkwhsW7ud7DANsWycGEL5ZOU50e.
firewall --service=ssh
authconfig --enableshadow --passalgo=sha512 --enablefingerprint
selinux --enforcing
timezone --utc Europe/Berlin
bootloader --location=mbr --driveorder=sda --append= LANG=en_US.UTF-8
SYSFONT=latarcyrheb-sun16 KEYBOARDTYPE=pc KEYTABLE=de-latin1-nodeadkeys
rhgb quiet
# The following is the partition information you requested
# Note that any partitions you deleted are not expressed
# here so unless you clear all partitions first, this is
# not guaranteed to work
clearpart --all --drives=sda --initlabel

part /boot --fstype=ext4 --size=200
part swap --grow --maxsize=256 --asprimary --size=1
part / --fstype=ext3 --grow --size=200

poweroff

%packages
@admin-tools
@base
@core
#...@editors
#...@fonts
@hardware-support
#...@input-methods
#...@online-docs
#...@text-internet
#gpgme
#gnupg2
#hdparm
#m17n-db-tamil
#m17n-db-gujarati
#m17n-db-kannada
#m17n-db-hindi
#m17n-db-oriya
#m17n-db-bengali
#m17n-contrib-sinhala
#m17n-db-assamese
#m17n-db-punjabi
#iok
#m17n-db-telugu
#tm17n-db-malayalam


-- 
Best regards, 

Gerhard Stenzel, 
---
IBM Deutschland Research  Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] bridge: Fix static-only DHCP configuration

2010-09-09 Thread Jiri Denemark
For static-only DHCP, i.e. with no range but at least one host
element within dhcp element, we have to add --dhcp-range IP,static
option to dnsmasq to actually enable the service. Without this option,
dnsmasq will not respond to DHCP requests.
---
 src/network/bridge_driver.c |   26 +++---
 1 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d6d3068..01d2171 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -394,6 +394,16 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
 int nbleases = 0;
 char *pidfileArg;
 char buf[1024];
+unsigned int ranges;
+
+/*
+ * For static-only DHCP, i.e. with no range but at least one host element,
+ * we have to add a special --dhcp-range option to enable the service in
+ * dnsmasq.
+ */
+ranges = network-def-nranges;
+if (!ranges  network-def-nhosts)
+ranges = 1;
 
 /*
  * NB, be careful about syntax for dnsmasq options in long format
@@ -424,11 +434,11 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
 /*2 + *//* --interface virbr0 */
 2 + /* --except-interface lo */
 2 + /* --listen-address 10.0.0.1 */
-(2 * network-def-nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */
+(2 * ranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */
 /* --dhcp-lease-max=xxx if needed */
 (network-def-nranges ? 1 : 0) +
 /* --dhcp-no-override if needed */
-(network-def-nranges ? 1 : 0) +
+(ranges ? 1 : 0) +
 /* --dhcp-hostsfile=/var/lib/dnsmasq/$NAME.hostsfile */
 (network-def-nhosts  0 ? 1 : 0) +
 /* --enable-tftp --tftp-root /srv/tftp */
@@ -496,12 +506,22 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
 nbleases += network-def-ranges[r].size;
 }
 
+if (!network-def-nranges  network-def-nhosts) {
+snprintf(buf, sizeof(buf), %s,static,
+ network-def-ipAddress);
+
+APPEND_ARG(*argv, i++, --dhcp-range);
+APPEND_ARG(*argv, i++, buf);
+}
+
 if (network-def-nranges  0) {
 snprintf(buf, sizeof(buf), --dhcp-lease-max=%d, nbleases);
 APPEND_ARG(*argv, i++, buf);
-APPEND_ARG(*argv, i++, --dhcp-no-override);
 }
 
+if (ranges)
+APPEND_ARG(*argv, i++, --dhcp-no-override);
+
 if (network-def-nhosts  0) {
 dnsmasqContext *dctx = dnsmasqContextNew(network-def-name, 
DNSMASQ_STATE_DIR);
 char *hostsfileArg;
-- 
1.7.2.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v3] buf: Fix possible infinite loop in EscapeString, VSnprintf

2010-09-09 Thread Cole Robinson
The current code will go into an infinite loop if the printf generated
string is = 1000, AND exactly 1 character smaller than the amount of free
space in the buffer. When this happens, we are dropped into the loop body,
but nothing will actually change, because count == (buf-size - buf-use - 1),
and virBufferGrow returns unchanged if count  (buf-size - buf-use)

Fix this by removing the '- 1' bit from 'size'. The *nprintf functions handle
the NULL byte for us anyways, so we shouldn't need to manually accommodate
for it.

Here's a bug where we are actually hitting this issue:
https://bugzilla.redhat.com/show_bug.cgi?id=602772

v2: Eric's improvements: while - if (), remove extra va_list variable,
make sure we report buffer error if snprintf fails

v3: Add tests/virbuftest which reproduces the infinite loop before this
patch, works correctly after
---
 src/util/buf.c |   70 ---
 tests/.gitignore   |1 +
 tests/Makefile.am  |7 -
 tests/virbuftest.c |   84 
 4 files changed, 137 insertions(+), 25 deletions(-)
 create mode 100644 tests/virbuftest.c

diff --git a/src/util/buf.c b/src/util/buf.c
index fc1217b..553e2a0 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -224,7 +224,7 @@ void
 virBufferVSprintf(const virBufferPtr buf, const char *format, ...)
 {
 int size, count, grow_size;
-va_list locarg, argptr;
+va_list argptr;
 
 if ((format == NULL) || (buf == NULL))
 return;
@@ -236,27 +236,38 @@ virBufferVSprintf(const virBufferPtr buf, const char 
*format, ...)
 virBufferGrow(buf, 100)  0)
 return;
 
-size = buf-size - buf-use - 1;
 va_start(argptr, format);
-va_copy(locarg, argptr);
-while (((count = vsnprintf(buf-content[buf-use], size, format,
-   locarg))  0) || (count = size - 1)) {
+
+size = buf-size - buf-use;
+if ((count = vsnprintf(buf-content[buf-use],
+   size, format, argptr))  0) {
+buf-error = 1;
+goto err;
+}
+
+/* Grow buffer if necessary and retry */
+if (count = size) {
 buf-content[buf-use] = 0;
-va_end(locarg);
+va_end(argptr);
+va_start(argptr, format);
 
-grow_size = (count  1000) ? count : 1000;
+grow_size = (count + 1  1000) ? count + 1 : 1000;
 if (virBufferGrow(buf, grow_size)  0) {
-va_end(argptr);
-return;
+goto err;
 }
 
-size = buf-size - buf-use - 1;
-va_copy(locarg, argptr);
+size = buf-size - buf-use;
+if ((count = vsnprintf(buf-content[buf-use],
+   size, format, argptr))  0) {
+buf-error = 1;
+goto err;
+}
 }
-va_end(argptr);
-va_end(locarg);
 buf-use += count;
-buf-content[buf-use] = '\0';
+
+err:
+va_end(argptr);
+return;
 }
 
 /**
@@ -336,23 +347,34 @@ virBufferEscapeString(const virBufferPtr buf, const char 
*format, const char *st
 
 if ((buf-use = buf-size) 
 virBufferGrow(buf, 100)  0) {
-VIR_FREE(escaped);
-return;
+goto err;
+}
+
+size = buf-size - buf-use;
+if ((count = snprintf(buf-content[buf-use], size,
+  format, (char *)escaped))  0) {
+buf-error = 1;
+goto err;
 }
 
-size = buf-size - buf-use - 1;
-while (((count = snprintf(buf-content[buf-use], size, format,
-  (char *)escaped))  0) || (count = size - 1)) {
+/* Grow buffer if necessary and retry */
+if (count = size) {
 buf-content[buf-use] = 0;
-grow_size = (count  1000) ? count : 1000;
+grow_size = (count + 1  1000) ? count + 1 : 1000;
 if (virBufferGrow(buf, grow_size)  0) {
-VIR_FREE(escaped);
-return;
+goto err;
+}
+size = buf-size - buf-use;
+
+if ((count = snprintf(buf-content[buf-use], size,
+  format, (char *)escaped))  0) {
+buf-error = 1;
+goto err;
 }
-size = buf-size - buf-use - 1;
 }
 buf-use += count;
-buf-content[buf-use] = '\0';
+
+err:
 VIR_FREE(escaped);
 }
 
diff --git a/tests/.gitignore b/tests/.gitignore
index 3f32939..e7c74c5 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -21,6 +21,7 @@ storagepoolxml2xmltest
 nodeinfotest
 statstest
 qparamtest
+virbuftest
 seclabeltest
 eventtest
 *.exe
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1dc7f66..dd59034 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -73,7 +73,7 @@ EXTRA_DIST =  \
qemuhelpdata
 
 check_PROGRAMS = virshtest conftest \
-nodeinfotest statstest qparamtest
+nodeinfotest statstest qparamtest virbuftest
 
 if WITH_XEN
 check_PROGRAMS += xml2sexprtest sexpr2xmltest \
@@ -151,6 +151,7 

Re: [libvirt] SVG sources for the libvirt wiki images is online

2010-09-09 Thread Andrew Cathrow
do you have the libvirt logo as svg? 


- Justin Clift jcl...@redhat.com wrote: 
 From: Justin Clift jcl...@redhat.com 
 To: Libvirt Developers Mailing List libvir-list@redhat.com 
 Sent: Thursday, September 9, 2010 9:56:48 AM GMT -05:00 US/Canada Eastern 
 Subject: [libvirt] SVG sources for the libvirt wiki images is online 
 
 Hi all, 
 
 The SVG sources for the images used in the libvirt wiki docs are online: 
 
 http://wiki.libvirt.org/page/SVGImages 
 
 That's a browsable list, still fairly disorganised (!), but contains a 
 visual list of the images on the wiki so far, with the link to their SVG 
 source. 
 
 They're all licensed under the Creative Commons Attribution–Share Alike 
 3.0 Unported license (CC-BY-SA): 
 
 http://creativecommons.org/licenses/by-sa/3.0/ 
 
 Feel welcome to use them in your wiki's, improve them, and so on. 
 
 Updates, ideas, and contributions welcome too of course. :) 
 
 Regards and best wishes, 
 
 Justin Clift 
 
 -- 
 libvir-list mailing list 
 libvir-list@redhat.com 
 https://www.redhat.com/mailman/listinfo/libvir-list 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] buf: Fix possible infinite loop in EscapeString, VSnprintf

2010-09-09 Thread Cole Robinson
On 09/02/2010 04:47 AM, Daniel P. Berrange wrote:
 On Wed, Sep 01, 2010 at 05:41:46PM -0400, Cole Robinson wrote:
 The current code will go into an infinite loop if the printf generated
 string is = 1000, AND exactly 1 character smaller than the amount of free
 space in the buffer. When this happens, we are dropped into the loop body,
 but nothing will actually change, because count == (buf-size - buf-use - 
 1),
 and virBufferGrow returns unchanged if count  (buf-size - buf-use)

 Fix this by removing the '- 1' bit from 'size'. The *nprintf functions handle
 the NULL byte for us anyways, so we shouldn't need to manually accomodate
 for it.

 Here's a bug where we are actually hitting this issue:
 https://bugzilla.redhat.com/show_bug.cgi?id=602772

 v2: Eric's improvements: while - if (), remove extra va_list variable,
 make sure we report buffer error if snprintf fails
 
 How about adding a unit test for the virBuffer APIs to verify all
 this stuff is working as designed. It is nicely self-contained
 code so we ought to be able to get 100% coverage of all codepaths
 and error conditions like this one
 
 Daniel

I sent an updated patch with a unittest that reproduces the infinite
loop as a start.

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] test: Don't overwrite storage volume target path and key

2010-09-09 Thread Cole Robinson
On 09/08/2010 04:41 PM, Matthias Bolte wrote:
 Only generate target path and key when they are not defined
 in the XML config.
 ---
  src/test/test_driver.c |   20 
  1 files changed, 12 insertions(+), 8 deletions(-)
 
 diff --git a/src/test/test_driver.c b/src/test/test_driver.c
 index 6c06cbc..9d22339 100644
 --- a/src/test/test_driver.c
 +++ b/src/test/test_driver.c
 @@ -706,17 +706,21 @@ static int testOpenVolumesForPool(xmlDocPtr xml,
  goto error;
  }
  
 -if (virAsprintf(def-target.path, %s/%s,
 -pool-def-target.path,
 -def-name) == -1) {
 -virReportOOMError();
 -goto error;
 +if (def-target.path == NULL) {
 +if (virAsprintf(def-target.path, %s/%s,
 +pool-def-target.path,
 +def-name) == -1) {
 +virReportOOMError();
 +goto error;
 +}
  }
  
 -def-key = strdup(def-target.path);
  if (def-key == NULL) {
 -virReportOOMError();
 -goto error;
 +def-key = strdup(def-target.path);
 +if (def-key == NULL) {
 +virReportOOMError();
 +goto error;
 +}
  }
  
  pool-def-allocation += def-allocation;

ACK

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] fix path to xen device statistics for newer kernels

2010-09-09 Thread Cole Robinson
On 09/08/2010 03:35 PM, Guido Günther wrote:
 Hi,
 it seems Xen changed the paths to the block device statistics in sysfs:
 from /sys/devices/xen-backend/vbd-%d-%d/statistics/%s to
 /sys/bus/xen-backend/devices/vbd-%d-%d/statistics/%s:
 
 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=596004
 
 Attached patch fixes this. O.k. to apply?
  -- Guido
 

 diff --git a/src/xen/block_stats.c b/src/xen/block_stats.c
 index 6e7a5c3..1d875f2 100644
 --- a/src/xen/block_stats.c
 +++ b/src/xen/block_stats.c
 @@ -118,6 +118,18 @@ read_bd_stat (int device, int domid, const char *str)
  int64_t r;
  
  snprintf (path, sizeof path,
 +  /sys/bus/xen-backend/devices/vbd-%d-%d/statistics/%s,
 +  domid, device, str);
 +r = read_stat (path);
 +if (r = 0) return r;
 +
 +snprintf (path, sizeof path,
 +  /sys/bus/xen-backend/devices/tap-%d-%d/statistics/%s,
 +  domid, device, str);
 +r = read_stat (path);
 +if (r = 0) return r;
 +
 +snprintf (path, sizeof path,
/sys/devices/xen-backend/vbd-%d-%d/statistics/%s,
domid, device, str);
  r = read_stat (path);

ACK if a brief comment is added explaining the situation.

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] SVG sources for the libvirt wiki images is online

2010-09-09 Thread Justin Clift

On 09/10/2010 12:11 AM, Andrew Cathrow wrote:

do you have the libvirt logo as svg?


Yep.  It's in a different git repository, direct link here:


http://libvirt.org/git/?p=libvirt-publican.git;a=blob_plain;f=en-US/images/image_left.svg;hb=HEAD

Hope that helps. :)

Regards and best wishes,

Justin Clift

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] Remove static binaries hack for DV environment

2010-09-09 Thread Daniel Veillard
  An old leftover from the early days, and it's getting into the way
now. Trivial, and uncontroversial as this should not affect anybody else
and clean this up, so I pushed this,

Daniel

diff --git a/configure.ac b/configure.ac
index 6c0fc69..a71f5e8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -237,13 +237,10 @@ AC_ARG_WITH([libvirtd],
   AC_HELP_STRING([--with-libvirtd], [add libvirtd support 
@:@default=yes@:@]),[],[with_libvirtd=yes])
 
 dnl
-dnl specific tests to setup DV devel environments with debug etc ...
+dnl in case someone want to build static binaries
+dnl STATIC_BINARIES=-static
 dnl
-if test ${LOGNAME} = veillard  test `pwd` = /u/veillard/libvirt ; 
then
-STATIC_BINARIES=-static
-else
-STATIC_BINARIES=
-fi
+STATIC_BINARIES=
 AC_SUBST([STATIC_BINARIES])
 
 dnl --enable-debug=(yes|no)

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] bridge: Fix static-only DHCP configuration

2010-09-09 Thread Daniel Veillard
On Thu, Sep 09, 2010 at 04:07:16PM +0200, Jiri Denemark wrote:
 For static-only DHCP, i.e. with no range but at least one host
 element within dhcp element, we have to add --dhcp-range IP,static
 option to dnsmasq to actually enable the service. Without this option,
 dnsmasq will not respond to DHCP requests.
 ---
  src/network/bridge_driver.c |   26 +++---
  1 files changed, 23 insertions(+), 3 deletions(-)
 
 diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
 index d6d3068..01d2171 100644
 --- a/src/network/bridge_driver.c
 +++ b/src/network/bridge_driver.c
 @@ -394,6 +394,16 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
  int nbleases = 0;
  char *pidfileArg;
  char buf[1024];
 +unsigned int ranges;
 +
 +/*
 + * For static-only DHCP, i.e. with no range but at least one host 
 element,
 + * we have to add a special --dhcp-range option to enable the service in
 + * dnsmasq.
 + */
 +ranges = network-def-nranges;
 +if (!ranges  network-def-nhosts)
 +ranges = 1;
  
  /*
   * NB, be careful about syntax for dnsmasq options in long format
 @@ -424,11 +434,11 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
  /*2 + *//* --interface virbr0 */
  2 + /* --except-interface lo */
  2 + /* --listen-address 10.0.0.1 */
 -(2 * network-def-nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */
 +(2 * ranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */
  /* --dhcp-lease-max=xxx if needed */
  (network-def-nranges ? 1 : 0) +
  /* --dhcp-no-override if needed */
 -(network-def-nranges ? 1 : 0) +
 +(ranges ? 1 : 0) +
  /* --dhcp-hostsfile=/var/lib/dnsmasq/$NAME.hostsfile */
  (network-def-nhosts  0 ? 1 : 0) +
  /* --enable-tftp --tftp-root /srv/tftp */
 @@ -496,12 +506,22 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
  nbleases += network-def-ranges[r].size;
  }
  
 +if (!network-def-nranges  network-def-nhosts) {
 +snprintf(buf, sizeof(buf), %s,static,
 + network-def-ipAddress);
 +
 +APPEND_ARG(*argv, i++, --dhcp-range);
 +APPEND_ARG(*argv, i++, buf);
 +}
 +
  if (network-def-nranges  0) {
  snprintf(buf, sizeof(buf), --dhcp-lease-max=%d, nbleases);
  APPEND_ARG(*argv, i++, buf);
 -APPEND_ARG(*argv, i++, --dhcp-no-override);
  }
  
 +if (ranges)
 +APPEND_ARG(*argv, i++, --dhcp-no-override);
 +
  if (network-def-nhosts  0) {
  dnsmasqContext *dctx = dnsmasqContextNew(network-def-name, 
 DNSMASQ_STATE_DIR);
  char *hostsfileArg;

ACK, looks fine,

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] test: Don't overwrite storage volume target path and key

2010-09-09 Thread Matthias Bolte
2010/9/9 Cole Robinson crobi...@redhat.com:
 On 09/08/2010 04:41 PM, Matthias Bolte wrote:
 Only generate target path and key when they are not defined
 in the XML config.
 ---
  src/test/test_driver.c |   20 
  1 files changed, 12 insertions(+), 8 deletions(-)

 diff --git a/src/test/test_driver.c b/src/test/test_driver.c
 index 6c06cbc..9d22339 100644
 --- a/src/test/test_driver.c
 +++ b/src/test/test_driver.c
 @@ -706,17 +706,21 @@ static int testOpenVolumesForPool(xmlDocPtr xml,
              goto error;
          }

 -        if (virAsprintf(def-target.path, %s/%s,
 -                        pool-def-target.path,
 -                        def-name) == -1) {
 -            virReportOOMError();
 -            goto error;
 +        if (def-target.path == NULL) {
 +            if (virAsprintf(def-target.path, %s/%s,
 +                            pool-def-target.path,
 +                            def-name) == -1) {
 +                virReportOOMError();
 +                goto error;
 +            }
          }

 -        def-key = strdup(def-target.path);
          if (def-key == NULL) {
 -            virReportOOMError();
 -            goto error;
 +            def-key = strdup(def-target.path);
 +            if (def-key == NULL) {
 +                virReportOOMError();
 +                goto error;
 +            }
          }

          pool-def-allocation += def-allocation;

 ACK

 - Cole


Thanks, pushed.

Matthias

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] QEMU json related fixes

2010-09-09 Thread Luiz Capitulino
Found these problems while running libvirt-tck with up to date libvirt
and qemu.git. Details in the patches.

Thanks.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] docs: add the app dev guide

2010-09-09 Thread Eric Blake

On 09/09/2010 03:21 AM, Justin Clift wrote:

Added a workable initial page for the libvirt Application
Development Guide, giving the online viewable options +
the downloadable pdf.

Added a link to the PDF to the main Downloads page, plus
neatened the html tags throughout the page as they
were a bit of a mess.



+
+pre
+a 
href=http://libvirt.org/git/?p=libvirt-appdev-guide.git;a=summary;http://libvirt.org/git/?p=libvirt-appdev-guide.git;a=summary/a/pre


Is it worth breaking up the long line?  Whitespace between a ... and 
the first character of the actual link is generally safe.



+pre
./autogen.sh --prefix=$HOME/usr


Should we mention --enable-compile-warnings=error here?

Other than that, ACK.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2/2] qemu: qemuMonitorJSONEjectMedia(): Fix arguments' type

2010-09-09 Thread Eric Blake

On 09/09/2010 03:05 PM, Luiz Capitulino wrote:

QMP in QEMU 0.13 has been fixed to enforce type correctness,
this means that boolean types must be true or false, not
integers.

Signed-off-by: Luiz Capitulinolcapitul...@redhat.com
---
  src/qemu/qemu_monitor_json.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bc19f23..d3ab25f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1353,7 +1353,7 @@ int qemuMonitorJSONEjectMedia(qemuMonitorPtr mon,
  int ret;
  virJSONValuePtr cmd = qemuMonitorJSONMakeCommand(eject,
   s:device, devname,
- i:force, 0,
+ b:force, 0,


ACK to both, and pushed.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v3] buf: Fix possible infinite loop in EscapeString, VSnprintf

2010-09-09 Thread Eric Blake

On 09/09/2010 08:09 AM, Cole Robinson wrote:

The current code will go into an infinite loop if the printf generated
string is= 1000, AND exactly 1 character smaller than the amount of free
space in the buffer. When this happens, we are dropped into the loop body,
but nothing will actually change, because count == (buf-size - buf-use - 1),
and virBufferGrow returns unchanged if count  (buf-size - buf-use)

Fix this by removing the '- 1' bit from 'size'. The *nprintf functions handle
the NULL byte for us anyways, so we shouldn't need to manually accommodate
for it.

Here's a bug where we are actually hitting this issue:
https://bugzilla.redhat.com/show_bug.cgi?id=602772

v2: Eric's improvements: while -  if (), remove extra va_list variable,
 make sure we report buffer error if snprintf fails

v3: Add tests/virbuftest which reproduces the infinite loop before this
 patch, works correctly after


I confirm that the only change in v3 is the addition of a test.


+static int testBufInfiniteLoop(const void *data ATTRIBUTE_UNUSED)
+{
+virBuffer bufinit = VIR_BUFFER_INITIALIZER;
+virBufferPtr buf =bufinit;
+char *addstr = NULL, *bufret = NULL;
+int ret = -1;
+const struct testInfo *info = data;
+
+virBufferAddChar(buf, 'a');
+if (buf-a != 1002 || buf-b != 1) {
+TEST_ERROR(Buf was not expected size, size=%d use=%d\n,
+   buf-a, buf-b);


This seems a bit fragile; it is inspecting the innards of virBuffer. 
Maybe add a comment here that any change to the innards of virBuffer may 
warrant a change to this test at the same time.  But having the test, 
even if it is fragile, is a good thing, so...


ACK.

We could add further tests to this file to cover more lines of code, 
such as: printing a string that needs escapes added, and actually 
verifying that the escaped string matches expectations; passing a bad 
format string (such as %$2s to trigger EINVAL, 
%*d%*d,131,1,131,1 to trigger EOVERFLOW) and verifying that error 
gets set.  But those can be in a separate patch; let's get the bug fix 
in now before 0.8.4.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list