Re: [libvirt] 'build' on FS pool now unconditionally formats?

2010-02-25 Thread Dave Allan
On 02/25/2010 08:25 AM, Daniel P. Berrange wrote: On Thu, Feb 25, 2010 at 01:19:54PM +0100, Daniel Veillard wrote: On Wed, Feb 24, 2010 at 03:51:45PM -0500, Cole Robinson wrote: Hi guys, Looking at the new FS pool build options and talking with Dave, I see that calling PoolBuild on an FS pool

Re: [libvirt] Odd implementation of libvirt qemudDomainSetMaxMemory

2010-02-25 Thread Cole Robinson
On 02/25/2010 06:15 PM, Chris Lalancette wrote: > Hey Cole, > In commit 09a33fd8a9cc85e5e1064168472bff7aecff15fb, you made it so that > qemudDomainSetMaxMemory() was hotplug only. However, with the changes you > pushed, this function no longer makes a lot of sense. It looks up the domain, >

[libvirt] Odd implementation of libvirt qemudDomainSetMaxMemory

2010-02-25 Thread Chris Lalancette
Hey Cole, In commit 09a33fd8a9cc85e5e1064168472bff7aecff15fb, you made it so that qemudDomainSetMaxMemory() was hotplug only. However, with the changes you pushed, this function no longer makes a lot of sense. It looks up the domain, checks to make sure it is active, and then if it is active

Re: [libvirt] [PATCH] Only build virDomainObjFormat if not building proxy.

2010-02-25 Thread Eric Blake
According to Chris Lalancette on 2/25/2010 12:48 PM: > While building under RHEL-5, I got a compile warning because > virDomainObjFormat was defined but not used. That came about > because in RHEL-5 we build with "#define PROXY", and > virDomainObjFormat is only used with !PROXY. Move the > defin

Re: [libvirt] [PATCH] Take Two - Fix domain restore for files on root-squash NFS.

2010-02-25 Thread Eric Blake
According to Laine Stump on 2/25/2010 3:19 PM: > Thanks once again for the informative review! (I've learned something > new about git, which isn't really surprising I supposed, since I > currently know so little ;-)) One of the things I love about open source is that everyone learns from each oth

Re: [libvirt] [PATCH] Take Two - Fix domain restore for files on root-squash NFS.

2010-02-25 Thread Laine Stump
Thanks once again for the informative review! (I've learned something new about git, which isn't really surprising I supposed, since I currently know so little ;-)) On 02/25/2010 12:34 PM, Eric Blake wrote: According to Laine Stump on 2/24/2010 1:58 PM: (This version incorporates the sugg

[libvirt] [PATCH] openvzGetVEID: don't leak (memory + file descriptor)

2010-02-25 Thread Jim Meyering
Coverity spotted the leaks. Here's a proposed fix. I didn't bother with a separate diagnostic for the unlikely event that we read a negative number from the pipe. Seems far-fetched enough not to bother, but it's easy to add, if anyone cares. >From f3439c7eae46681eacf5b469a6f0e22cb8fec1b4 Mon Sep

[libvirt] [PATCH] Only build virDomainObjFormat if not building proxy.

2010-02-25 Thread Chris Lalancette
While building under RHEL-5, I got a compile warning because virDomainObjFormat was defined but not used. That came about because in RHEL-5 we build with "#define PROXY", and virDomainObjFormat is only used with !PROXY. Move the define. Signed-off-by: Chris Lalancette --- src/conf/domain_conf.

Re: [libvirt] [PATCH] openvzGetVEID: don't leak (memory + file descriptor)

2010-02-25 Thread Eric Blake
According to Jim Meyering on 2/25/2010 12:01 PM: >> You're still keeping with fscanf. > > Yes. There are plenty of bigger fish to fry, for now ;-) > As it is, this is more of a rearrangement than I generally > prefer "just to fix a leak". Indeed, I have no problem with checking in your patch as-

Re: [libvirt] [PATCH] openvzGetVEID: don't leak (memory + file descriptor)

2010-02-25 Thread Jim Meyering
Eric Blake wrote: > According to Jim Meyering on 2/25/2010 11:30 AM: > > ACK on plugging the leak. However,... Thanks for the review. >> @@ -979,18 +980,12 @@ int openvzGetVEID(const char *name) { >> return -1; >> } >> >> -if (fscanf(fp, "%d\n", &veid ) != 1) { >> +ok = fsc

Re: [libvirt] [PATCH] openvzGetVEID: don't leak (memory + file descriptor)

2010-02-25 Thread Eric Blake
According to Jim Meyering on 2/25/2010 11:30 AM: ACK on plugging the leak. However,... > @@ -979,18 +980,12 @@ int openvzGetVEID(const char *name) { > return -1; > } > > -if (fscanf(fp, "%d\n", &veid ) != 1) { > +ok = fscanf(fp, "%d\n", &veid ) == 1; You're still keeping

Re: [libvirt] [PATCH] Take Two - Fix domain restore for files on root-squash NFS.

2010-02-25 Thread Eric Blake
According to Laine Stump on 2/24/2010 1:58 PM: > (This version incorporates the suggestions from Jim Meyering and Eric Blake) I guess that line is okay to leave in the commit message. If it had been me, though, I would have left it out of the commit log, and just inserted it between the --- and d

Re: [libvirt] 'build' on FS pool now unconditionally formats?

2010-02-25 Thread Cole Robinson
On 02/25/2010 09:55 AM, Cole Robinson wrote: > On 02/25/2010 08:25 AM, Daniel P. Berrange wrote: >> On Thu, Feb 25, 2010 at 01:19:54PM +0100, Daniel Veillard wrote: >>> On Wed, Feb 24, 2010 at 03:51:45PM -0500, Cole Robinson wrote: Hi guys, Looking at the new FS pool build options an

Re: [libvirt] 'build' on FS pool now unconditionally formats?

2010-02-25 Thread Cole Robinson
On 02/25/2010 08:25 AM, Daniel P. Berrange wrote: > On Thu, Feb 25, 2010 at 01:19:54PM +0100, Daniel Veillard wrote: >> On Wed, Feb 24, 2010 at 03:51:45PM -0500, Cole Robinson wrote: >>> Hi guys, >>> >>> Looking at the new FS pool build options and talking with Dave, I see that >>> calling PoolBuil

Re: [libvirt] [PATCH 0/3] Refactor XML parsing code

2010-02-25 Thread Jiri Denemark
> > Almost identical XML parsing code for either strings or files was copied > > 15 times. Since it was needed on two other places for parsing CPU XML > > strings with correct error reporting, I decided to factor the parsing code > > out to xml.c and used this one implementation in all other places

Re: [libvirt] avoid "make rpm" failure in doc/

2010-02-25 Thread Jim Meyering
Jim Meyering wrote: > "make rpm" fails like this: > > Rebuilding the HTML pages from the XML API > make[3]: *** No rule to make target `html/libvirt-libvirt.html', needed > by `distdir'. Stop. > make[3]: *** Waiting for unfinished jobs > Validating the resulting XHTML pages >

[libvirt] [PATCH] build: avoid warning about unused variables

2010-02-25 Thread Jim Meyering
>From 8ddff3e6cdccc2b4289509c6941b43f2ddf8e643 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Thu, 25 Feb 2010 14:19:33 +0100 Subject: [PATCH] build: avoid warning about unused variables * tools/virsh.c (cmdCPUBaseline): Remove declarations of unused variables, p and cur. --- tools/virsh.c

[libvirt] avoid "make rpm" failure in doc/

2010-02-25 Thread Jim Meyering
"make rpm" fails like this: Rebuilding the HTML pages from the XML API make[3]: *** No rule to make target `html/libvirt-libvirt.html', needed by `distdir'. Stop. make[3]: *** Waiting for unfinished jobs Validating the resulting XHTML pages These two patches fix that: (thoug

Re: [libvirt] 'build' on FS pool now unconditionally formats?

2010-02-25 Thread Daniel P. Berrange
On Thu, Feb 25, 2010 at 01:19:54PM +0100, Daniel Veillard wrote: > On Wed, Feb 24, 2010 at 03:51:45PM -0500, Cole Robinson wrote: > > Hi guys, > > > > Looking at the new FS pool build options and talking with Dave, I see that > > calling PoolBuild on an FS pool now unconditionally calls mkfs. This

[libvirt] [PATCH] build: avoid non-srcdir "make distcheck" failures (CLEANFILES)

2010-02-25 Thread Jim Meyering
A quick review won't hurt, but this doesn't really need one. >From 9ac893a1342b31ef7df6fc57c03767c49ef98d5b Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Thu, 25 Feb 2010 09:28:51 +0100 Subject: [PATCH 1/4] build: avoid non-srcdir "make distcheck" failures (CLEANFILES) * docs/Makefile.am (M

Re: [libvirt] [PATCH] qemu: Report binary path if error parsing -help

2010-02-25 Thread Daniel Veillard
On Wed, Feb 24, 2010 at 12:55:19PM -0500, Cole Robinson wrote: > > Signed-off-by: Cole Robinson > --- > src/qemu/qemu_conf.c | 14 -- > src/qemu/qemu_conf.h |3 ++- > tests/qemuhelptest.c |3 ++- > 3 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/src/qemu

Re: [libvirt] [PATCH] remote: Improve daemon startup error reporting

2010-02-25 Thread Daniel Veillard
On Wed, Feb 24, 2010 at 12:55:18PM -0500, Cole Robinson wrote: > If I toggle enable_tcp in libvirtd.conf and add --listen in > /etc/init.d/libvirtd, I get the unhelpful error: > > Starting libvirtd daemon: error: Unable to initialize network sockets. > > Running without --daemon provides much mor

Re: [libvirt] [PATCH] virsh: Show errors reported by nonAPI functions

2010-02-25 Thread Daniel Veillard
On Wed, Feb 24, 2010 at 12:55:17PM -0500, Cole Robinson wrote: > Only API calls trigger the error callback, which is required for > proper virsh error reporting. Since we use non API functions from > util/, make sure we properly report these errors. > > Fixes lack of error message from 'virsh crea

Re: [libvirt] 'build' on FS pool now unconditionally formats?

2010-02-25 Thread Daniel Veillard
On Wed, Feb 24, 2010 at 03:51:45PM -0500, Cole Robinson wrote: > Hi guys, > > Looking at the new FS pool build options and talking with Dave, I see that > calling PoolBuild on an FS pool now unconditionally calls mkfs. This is really > bad when mixed with virt-manager: previously, we assumed the F

Re: [libvirt] [PATCH 0/3] Refactor XML parsing code

2010-02-25 Thread Daniel Veillard
On Wed, Feb 24, 2010 at 11:07:31PM +0100, Jiri Denemark wrote: > Almost identical XML parsing code for either strings or files was copied > 15 times. Since it was needed on two other places for parsing CPU XML > strings with correct error reporting, I decided to factor the parsing code > out to xml