Re: [libvirt] RFC: Setting a permanent MAC address for virtual network bridges

2010-12-07 Thread Daniel P. Berrange
On Tue, Dec 07, 2010 at 01:46:40AM -0500, Laine Stump wrote:
> This is in reference to the following bugs that are filed against
> RHEL, but applicable to all Linux distros:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=560994
> https://bugzilla.redhat.com/show_bug.cgi?id=609463
> 
> "MAC address of virtual network bridge changes when TAP devices are
> added/removed"
> 
> The problem is that the MAC address of a bridge that hasn't had its
> address specifically set, will be set to the MAC address of the
> first interface attached to it. If that interface later goes away,
> it will be set to the MAC address of another attached interface.
> 
> Daniel's comments in the bug 1) state that the MAC address of a
> bridge device must match the MAC address of one of the attached
> interfaces, or no traffic will pass, and 2) suggest creating a dummy
> interface that is created and used purely to set a fixed MAC address
> that won't go away when any guest is destroyed (because it's not
> associated with any guest.
> 
> I did some quick experimentation and found that:
> 
> 
> 1) It is possible to set a bridge's MAC address to something that
> isn't the address of an attached interface (although the range of
> numbers seems to be limited, at least when setting it using "ip link
> set virbr0 address xx:xx:xx:xx:xx:xx - anything starting with less
> than "B2" was rejected).
> 
> 2) When a bridge's MAC address is set in that way, as Daniel says,
> no traffic passes. (Why is that, anyway?)

A bridge device *must* have a MAC address that machines one of
its enslaved devices. It can't have its own MAC address. The
bridge will automatically obtain the MAC of the first interface
enslaved normally.

> 3) If I create a tap device, set a MAC address on it, attach it to
> the bridge, and make sure the bridge is assigned that MAC address,
> traffic does pass, and when guests are destroyed and restarted, the
> MAC address of the bridge doesn't change.

You need to make sure the MAC address of the special TAP device is
numerically smaller than the MAC address of any normal VM TAP
device. This is fairly easy, since we now ensure all TAP devices
for VMs have 0xFE for their high byte. So the special device can
just have any value smaller than 0xFE (for values of 'any' which
are careful not to select a multicast MAC).

> So it looks like this is a viable way to solve the problem. Before
> digging into it, though, I thought I should ask if there are any
> alternate ideas, or problems that anyone sees with this approach.
> 
> Also, I'm wondering where the name and MAC address of the dummy
> interfaces should come from, and whether I need to worry about
> storing them in the network XML. So far, this is what I think on
> those topics:

You could make a case for including the MAC address in
the XML, describing it as the MAC address of the router. We
should definitely not include the interface name because that
is a Linux specific implementation details.

> 1) the MAC address can be autogenerated if it's not in the XML, but
> needs to be saved in the XML so that it's persistent across libvirtd
> restarts.
> 
> 2) the dummy interface name can just be some derivation of the
> bridge device name ("virbr0-mac"?), and probably doesn't need to be
> saved in the XML.

Agreed.

> (Also, I just noticed the code that sets the first byte of the MAC
> address of all tap interfaces being attached to virtual network
> bridges to 0xFE. What's the story behind that? Does the bridge
> always take the numerically lowest MAC that's currently connected?)

Yes. This is to guarnetee that all guest TAP devices have a MAC
that is higher than any physical NIC (no NIC vendor has a prefix
assigned with 0xFE or 0xFF)

Daniel

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


Re: [libvirt] [PATCH] virsh: Remove redundant commands in group domain

2010-12-07 Thread Justin Clift
On 07/12/2010, at 6:25 PM, Osier Yang wrote:
> "echo" is already in group "virsh", "freecell" and "hostname" are
> already in group "host", so remove them from group "domain"
> 
> * tools/virsh.c
> ---

ACK.

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


Re: [libvirt] RFC: Setting a permanent MAC address for virtual network bridges

2010-12-07 Thread Laine Stump

On 12/07/2010 05:23 AM, Daniel P. Berrange wrote:

On Tue, Dec 07, 2010 at 01:46:40AM -0500, Laine Stump wrote:

2) When a bridge's MAC address is set in that way, as Daniel says,
no traffic passes. (Why is that, anyway?)

A bridge device *must* have a MAC address that machines one of
its enslaved devices. It can't have its own MAC address. The
bridge will automatically obtain the MAC of the first interface
enslaved normally.



Yeah, I understand that *is* the case. I'm just curious what the logic 
behind that is. It's certainly not the case for an actual hardware bridge...




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


Re: [libvirt] [PATCH] virsh: Remove redundant commands in group domain

2010-12-07 Thread Eric Blake
On 12/07/2010 07:20 AM, Justin Clift wrote:
> On 07/12/2010, at 6:25 PM, Osier Yang wrote:
>> "echo" is already in group "virsh", "freecell" and "hostname" are
>> already in group "host", so remove them from group "domain"
>>
>> * tools/virsh.c
>> ---
> 
> ACK.

Pushed.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] snapshots += domain description?

2010-12-07 Thread Philipp Hahn
Hello,

I just encountered a problem with the current snapshot implementation for 
qemu/kvm: To revert back to a snapshot, the domain description must closely 
match the domain description when the snapshot was created. If the ram-size 
doesn't match or the VM now contains fewer CPUs or contains additional disks, 
kvm will either not load the snapshot or the machine will be broken 
afterwarts.
Is this a known problem?

Without looking into the implementation details I think saving the full domain 
description instead of just a reference to the domain uuid as part of the 
snapshot description would work better. Or am I supposed to save the domain 
description myself in addition the the data saved 
in /var/lib/libvirt/qemu/snapshot/$DOMAIN_NAME/$SNAPSHOT_NAME.xml. I think 
this is very important feature, because when you - for example - notice that 
you need additional disk space and would like to include an additional block 
device, this is your ideal time to take a snapshot you can revert to when you 
do something wrong.

Looking at VMware I see that reverting a VM there will not only revert the 
disks and ram back to the saved state, but also the description including 
name, ram size, etc.

Any comment or advise is appreciated.

Sincerely
Philipp Hahn
-- 
Philipp Hahn   Open Source Software Engineer  h...@univention.de   
Univention GmbHLinux for Your Businessfon: +49 421 22 232- 0
Mary-Somerville-Str.1  28359 Bremen   fax: +49 421 22 232-99
http://www.univention.de


signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] spec: do not start libvirt-guests if that service is off

2010-12-07 Thread Dan Kenigsberg
starting a service during rpm installation is impolite. It is even worse if done
during upgrade, for a service that was explicitly turned off.
---
 libvirt.spec.in |8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 48453d7..e0cab78 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -700,9 +700,11 @@ fi
 /sbin/ldconfig
 /sbin/chkconfig --add libvirt-guests
 if [ $1 -ge 1 ]; then
-# this doesn't do anything but allowing for libvirt-guests to be
-# stopped on the first shutdown
-/sbin/service libvirt-guests start > /dev/null 2>&1 || true
+if /sbin/chkconfig --list libvirt-guests | /bin/grep -q :on ; then
+# this doesn't do anything but allowing for libvirt-guests to be
+# stopped on the first shutdown
+/sbin/service libvirt-guests start > /dev/null 2>&1 || true
+fi
 fi
 
 %postun client -p /sbin/ldconfig
-- 
1.7.3.2

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


Re: [libvirt] [PATCHv2 1/5] qemu: avoid adding "" in smbios arguments

2010-12-07 Thread Eric Blake
On 12/05/2010 12:57 AM, Daniel Veillard wrote:
> On Fri, Dec 03, 2010 at 02:56:14PM -0700, Eric Blake wrote:
>> The log lists things like -smbios type=1,vendor="Red Hat", which
>> is great for shell parsing, but not so great when you realize that
>> execve() then passes those literal "" on as part of the command
>> line argument, such that qemu sets SMBIOS with extra literal quotes.
> 
>   Hum, I was afraid that QEmu parsing would fail in case of spaces if
> there is no quote, but if you checked this, sure !

What's happening here is that we are building up execve arguments, and
supplying roughly:

"qemu" "-smbios" "type=0,vendor=\"Red Hat\",version=\"Fedora 14\""

instead of the intended:

"qemu" "-smbios" "type=0,vendor=Red Hat,version=Fedora 14"

Although qemu uses a hand-rolled loop instead of getsubopt(), it looks
like qemu is using the same algorithm as getsubopt, where it parses
everything between '=' and ',', including spaces, as the subopt
argument.  At any rate, yes, I did test this; the only thing you can't
pass through qemu's -smbios is a literal comma, but that's already
excluded from our domain.rng schema. :)

>   ACK,

Thanks; I've pushed 1 through 4; I'm waiting to push 5 until after my
virCommand buffer patches have been ACK'd, so as to avoid any question
of any potential NULL dereferences due to the virCommandSetOutputBuffer
calls.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/4] tests: Fix code formating in commandtest

2010-12-07 Thread Jiri Denemark
> >  tests/commandtest.c |   55 
> > +-
> >  1 files changed, 36 insertions(+), 19 deletions(-)
> 
> ACK: No semantic changes.

Thanks, I pushed this and patch 4.

Jirka

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


Re: [libvirt] [PATCHv2 1/2] command: improve allocation failure reporting

2010-12-07 Thread Jiri Denemark
> * src/util/command.c (virCommandAddEnvString): Remove duplicate
> code.
> (virCommandToString, virCommandRun, virCommandRunAsync)
> (virCommandWait): Report NULL command as ENOMEM, not invalid
> usage.
> Reported by Jiri Denemark.
> ---
> 
> New patch.  Fixes the root cause, so that patch 2 can
> be smaller...

Yeah, makes sense. I didn't realize the expected behavior was not to require
error checking even after virCommandNew*

> diff --git a/src/util/command.c b/src/util/command.c
> index 38d462b..c0520ec 100644
> --- a/src/util/command.c
> +++ b/src/util/command.c
> @@ -292,9 +292,6 @@ virCommandAddEnvPair(virCommandPtr cmd, const char *name, 
> const char *value)
>  void
>  virCommandAddEnvString(virCommandPtr cmd, const char *str)
>  {
> -if (!cmd || cmd->has_error)
> -return;
> -
>  char *env;
> 
>  if (!cmd || cmd->has_error)

Eh.

> @@ -710,13 +707,13 @@ virCommandToString(virCommandPtr cmd)
> 
>  /* Cannot assume virCommandRun will be called; so report the error
>   * now.  If virCommandRun is called, it will report the same error. */
> -if (!cmd || cmd->has_error == -1) {
> -virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
> -_("invalid use of command API"));
> +if (!cmd ||cmd->has_error == ENOMEM) {
> +virReportOOMError();
>  return NULL;
>  }
> -if (cmd->has_error == ENOMEM) {
> -virReportOOMError();
> +if (cmd->has_error) {
> +virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
> +_("invalid use of command API"));
>  return NULL;
>  }

I checked has_error is either 0, -1, or ENOMEM so we are not hiding any error
here.

ACK

Jirka

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


Re: [libvirt] [PATCHv2 2/2] tests: fix leaks in commandtest

2010-12-07 Thread Jiri Denemark
> Most leaks could only occur on error cleanup paths.
> ---
> 
> Respin of Jiri's 2/4 v1 patch, now much smaller by relying
> on guaranteed semantics.
> 
> Jiri's patch 1 and 4 of the v1 series are still okay to apply
> as-is, but patch 3/4 needs rebasing onto this patch.
> 
>  tests/commandtest.c |   72 +++---
>  1 files changed, 45 insertions(+), 27 deletions(-)

ACK

Jirka

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


[libvirt] [PATCH] qemu: Enable disabled debug messages

2010-12-07 Thread Jiri Denemark
---
 src/qemu/qemu_conf.c |2 +-
 src/qemu/qemu_conf.h |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index d81e6cc..08c084b 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1449,7 +1449,7 @@ int qemudParseHelpStr(const char *qemu,
 
 *flags = qemudComputeCmdFlags(help, *version, *is_kvm, *kvm_version);
 
-qemudDebug("Version %u.%u.%u, cooked version %u, flags %u",
+qemudDebug("Version %u.%u.%u, cooked version %u, flags 0x%llx",
major, minor, micro, *version, *flags);
 if (*kvm_version)
 qemudDebug("KVM version %d detected", *kvm_version);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a1556cb..50938f5 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -44,7 +44,7 @@
 # include "macvtap.h"
 # include "command.h"
 
-# define qemudDebug(fmt, ...) do {} while(0)
+# define qemudDebug DEBUG
 
 # define QEMUD_CPUMASK_LEN CPU_SETSIZE
 
-- 
1.7.3.2

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


Re: [libvirt] [PATCH] qemu: Enable disabled debug messages

2010-12-07 Thread Daniel P. Berrange
On Tue, Dec 07, 2010 at 06:24:45PM +0100, Jiri Denemark wrote:
> ---
>  src/qemu/qemu_conf.c |2 +-
>  src/qemu/qemu_conf.h |2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index d81e6cc..08c084b 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1449,7 +1449,7 @@ int qemudParseHelpStr(const char *qemu,
>  
>  *flags = qemudComputeCmdFlags(help, *version, *is_kvm, *kvm_version);
>  
> -qemudDebug("Version %u.%u.%u, cooked version %u, flags %u",
> +qemudDebug("Version %u.%u.%u, cooked version %u, flags 0x%llx",
> major, minor, micro, *version, *flags);
>  if (*kvm_version)
>  qemudDebug("KVM version %d detected", *kvm_version);
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index a1556cb..50938f5 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -44,7 +44,7 @@
>  # include "macvtap.h"
>  # include "command.h"
>  
> -# define qemudDebug(fmt, ...) do {} while(0)
> +# define qemudDebug DEBUG
>  
>  # define QEMUD_CPUMASK_LEN CPU_SETSIZE

It'd be preferrable to kill qemudDebug and just use
VIR_DEBUG directly

I'm already doing this in the daemon/ code I'm
currently re-writing. There's just a few instances
left in src/qemu/

Daniel

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


Re: [libvirt] [PATCH 3/4] tests: Fix commandtest in VPATH build

2010-12-07 Thread Eric Blake
On 12/06/2010 05:03 AM, Jiri Denemark wrote:
> ---
>  tests/commandtest.c |6 +-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/tests/commandtest.c b/tests/commandtest.c
> index 48c6335..e8decd3 100644
> --- a/tests/commandtest.c
> +++ b/tests/commandtest.c
> @@ -587,12 +587,15 @@ cleanup:
>  static int test15(const void *unused ATTRIBUTE_UNUSED)
>  {
>  virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper");
> +char *cwd = NULL;
>  int ret = -1;
>  
>  if (!cmd)
>  return -1;
>  
> -virCommandSetWorkingDirectory(cmd, abs_builddir "/commanddata");
> +if (virAsprintf(&cwd, "%s/commanddata", abs_srcdir) < 0)
> +goto cleanup;
> +virCommandSetWorkingDirectory(cmd, cwd);

ACK; I've gone ahead and pushed my rewrite of patch 2 in your series, as
well as the rebase of this patch 3 from your series.

I still haven't tried running commandtest under valgrind to see if there
are any other leaks to plug (so far, it's just been patches by
inspection and prove that the testsuite still passes), but I hope to get
to that point as well.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Enable disabled debug messages

2010-12-07 Thread Eric Blake
On 12/07/2010 10:24 AM, Jiri Denemark wrote:
> ---
>  src/qemu/qemu_conf.c |2 +-
>  src/qemu/qemu_conf.h |2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index d81e6cc..08c084b 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1449,7 +1449,7 @@ int qemudParseHelpStr(const char *qemu,
>  
>  *flags = qemudComputeCmdFlags(help, *version, *is_kvm, *kvm_version);
>  
> -qemudDebug("Version %u.%u.%u, cooked version %u, flags %u",
> +qemudDebug("Version %u.%u.%u, cooked version %u, flags 0x%llx",
> major, minor, micro, *version, *flags);
>  if (*kvm_version)
>  qemudDebug("KVM version %d detected", *kvm_version);
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index a1556cb..50938f5 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -44,7 +44,7 @@
>  # include "macvtap.h"
>  # include "command.h"
>  
> -# define qemudDebug(fmt, ...) do {} while(0)
> +# define qemudDebug DEBUG

ACK.  Hmm; should we rename the use of qemudDebug in daemon/libvirtd.c
to something more appropriate, as a follow-on cleanup?

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Just in case it's useful: virsh commands, their version, and their commits

2010-12-07 Thread Justin Clift
Hi all,

Just put a (temporary) page on the wiki showing all of the virsh commands, the 
version of libvirt each was introduced in, and the commit that did it.

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

It's very "raw data" style, and at some point in the next week the info will 
get transferred somewhere better (Virsh Command Reference).  But figured other 
people out there might have a use for it (no idea).

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] qemu: Enable disabled debug messages

2010-12-07 Thread Eric Blake
On 12/07/2010 10:40 AM, Eric Blake wrote:
>> +++ b/src/qemu/qemu_conf.h
>> @@ -44,7 +44,7 @@
>>  # include "macvtap.h"
>>  # include "command.h"
>>  
>> -# define qemudDebug(fmt, ...) do {} while(0)
>> +# define qemudDebug DEBUG
> 
> ACK.  Hmm; should we rename the use of qemudDebug in daemon/libvirtd.c
> to something more appropriate, as a follow-on cleanup?

On the other hand, I agree with danpb's NACK to just rewrite the few
remaining uses directly into VIR_DEBUG, rather than keeping the name
qemudDebug around.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv2] qemu: Enable disabled debug messages

2010-12-07 Thread Jiri Denemark
---
 src/qemu/qemu_conf.c |8 
 src/qemu/qemu_conf.h |2 --
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index d81e6cc..e5d0206 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1449,12 +1449,12 @@ int qemudParseHelpStr(const char *qemu,
 
 *flags = qemudComputeCmdFlags(help, *version, *is_kvm, *kvm_version);
 
-qemudDebug("Version %u.%u.%u, cooked version %u, flags %u",
-   major, minor, micro, *version, *flags);
+VIR_DEBUG("Version %u.%u.%u, cooked version %u, flags 0x%llx",
+  major, minor, micro, *version, *flags);
 if (*kvm_version)
-qemudDebug("KVM version %d detected", *kvm_version);
+VIR_DEBUG("KVM version %d detected", *kvm_version);
 else if (*is_kvm)
-qemudDebug("qemu-kvm version %u.%u.%u detected", major, minor, micro);
+VIR_DEBUG("qemu-kvm version %u.%u.%u detected", major, minor, micro);
 
 return 0;
 
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a1556cb..71318bf 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -44,8 +44,6 @@
 # include "macvtap.h"
 # include "command.h"
 
-# define qemudDebug(fmt, ...) do {} while(0)
-
 # define QEMUD_CPUMASK_LEN CPU_SETSIZE
 
 /* Internal flags to keep track of qemu command line capabilities */
-- 
1.7.3.2

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


[libvirt] [PATCH 0/2] rbd network disk support

2010-12-07 Thread Josh Durgin
Here are patches on top of Kazutaka's v2 to add RBD support and fix
some general network disk problems. There's also a test for each type
of network disk.

Josh Durgin (2):
  qemu: Add RBD support and some network disk fixes
  tests: Add tests for network disks

 docs/schemas/domain.rng|   11 ++-
 src/conf/domain_conf.c |   25 +++-
 src/conf/domain_conf.h |1 +
 src/qemu/qemu_conf.c   |  143 ++--
 tests/qemuargv2xmltest.c   |3 +
 .../qemuxml2argv-disk-drive-network-nbd.args   |1 +
 .../qemuxml2argv-disk-drive-network-nbd.xml|   32 +
 .../qemuxml2argv-disk-drive-network-rbd.args   |1 +
 .../qemuxml2argv-disk-drive-network-rbd.xml|   34 +
 .../qemuxml2argv-disk-drive-network-sheepdog.args  |1 +
 .../qemuxml2argv-disk-drive-network-sheepdog.xml   |   32 +
 tests/qemuxml2argvtest.c   |6 +
 12 files changed, 275 insertions(+), 15 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml


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


[libvirt] [PATCH 1/2] qemu: Add RBD support and some network disk fixes

2010-12-07 Thread Josh Durgin

Changes common to all network disks:
-Make source name optional in the domain schema, since NBD doesn't use it
-Add a hostName type to the domain schema, and use it instead of genericName, 
which doesn't include .
-Don't leak host names or ports
-Set the source protocol in qemuParseCommandline

Signed-off-by: Josh Durgin 
---
 docs/schemas/domain.rng |   11 +++-
 src/conf/domain_conf.c  |   25 +++-
 src/conf/domain_conf.h  |1 +
 src/qemu/qemu_conf.c|  143 ---
 4 files changed, 165 insertions(+), 15 deletions(-)

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 4463884..51aae14 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -626,11 +626,13 @@
 sheepdog
   
 
-
+
+  
+
 
   
 
-  
+  
 
 
   
@@ -2024,6 +2026,11 @@
   1
 
   
+  
+
+  [a-zA-Z0-9\.\-]+
+
+  
   
 
   -1
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5e2422b..6b4320a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -508,21 +508,34 @@ void virDomainInputDefFree(virDomainInputDefPtr def)

 void virDomainDiskDefFree(virDomainDiskDefPtr def)
 {
+unsigned int i;
+
 if (!def)
 return;

 VIR_FREE(def->serial);
 VIR_FREE(def->src);
-VIR_FREE(def->hosts);
 VIR_FREE(def->dst);
 VIR_FREE(def->driverName);
 VIR_FREE(def->driverType);
 virStorageEncryptionFree(def->encryption);
 virDomainDeviceInfoClear(&def->info);

+for (i = 0 ; i < def->nhosts ; i++)
+virDomainDiskHostDefFree(&def->hosts[i]);
+
 VIR_FREE(def);
 }

+void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def)
+{
+if (!def)
+return;
+
+VIR_FREE(def->name);
+VIR_FREE(def->port);
+}
+
 void virDomainControllerDefFree(virDomainControllerDefPtr def)
 {
 if (!def)
@@ -1643,7 +1656,12 @@ virDomainDiskDefParseXML(virCapsPtr caps,
  protocol);
 goto error;
 }
-source = virXMLPropString(cur, "name");
+if (!(source = virXMLPropString(cur, "name")) &&
+def->protocol != VIR_DOMAIN_DISK_PROTOCOL_NBD) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing name for disk 
source"));
+goto error;
+}
 host = cur->children;
 while (host != NULL) {
 if (host->type == XML_ELEMENT_NODE &&
@@ -1876,8 +1894,7 @@ cleanup:
 VIR_FREE(target);
 VIR_FREE(source);
 while (nhosts > 0) {
-VIR_FREE(hosts[nhosts - 1].name);
-VIR_FREE(hosts[nhosts - 1].port);
+virDomainDiskHostDefFree(&hosts[nhosts - 1]);
 nhosts--;
 }
 VIR_FREE(hosts);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6c97289..c1e39ba 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1070,6 +1070,7 @@ virDomainObjPtr virDomainFindByName(const 
virDomainObjListPtr doms,
 void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def);
 void virDomainInputDefFree(virDomainInputDefPtr def);
 void virDomainDiskDefFree(virDomainDiskDefPtr def);
+void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def);
 void virDomainControllerDefFree(virDomainControllerDefPtr def);
 void virDomainFSDefFree(virDomainFSDefPtr def);
 void virDomainNetDefFree(virDomainNetDefPtr def);
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 55e193f..d1368dc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -4010,6 +4010,8 @@ qemudBuildCommandLine(virConnectPtr conn,
 int last_good_net = -1;
 bool hasHwVirt = false;
 virCommandPtr cmd;
+bool has_rbd_hosts = false;
+virBuffer rbd_hosts = VIR_BUFFER_INITIALIZER;

 uname_normalize(&ut);

@@ -4550,6 +4552,7 @@ qemudBuildCommandLine(virConnectPtr conn,
 int bootable = 0;
 virDomainDiskDefPtr disk = def->disks[i];
 int withDeviceArg = 0;
+int j;

 /* Unless we have -device, then USB disks need special
handling */
@@ -4599,6 +4602,27 @@ qemudBuildCommandLine(virConnectPtr conn,
 virCommandAddArg(cmd, optstr);
 VIR_FREE(optstr);

+if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK &&
+disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) {
+for (j = 0 ; j < disk->nhosts ; j++) {
+if (!has_rbd_hosts) {
+virBufferAddLit(&rbd_hosts, "-m ");
+has_rbd_hosts = true;
+   

[libvirt] [PATCH 2/2] tests: Add tests for network disks

2010-12-07 Thread Josh Durgin

Signed-off-by: Josh Durgin 
---
 tests/qemuargv2xmltest.c   |3 ++
 .../qemuxml2argv-disk-drive-network-nbd.args   |1 +
 .../qemuxml2argv-disk-drive-network-nbd.xml|   32 ++
 .../qemuxml2argv-disk-drive-network-rbd.args   |1 +
 .../qemuxml2argv-disk-drive-network-rbd.xml|   34 
 .../qemuxml2argv-disk-drive-network-sheepdog.args  |1 +
 .../qemuxml2argv-disk-drive-network-sheepdog.xml   |   32 ++
 tests/qemuxml2argvtest.c   |6 +++
 8 files changed, 110 insertions(+), 0 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml

diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index d941b0b..adff05a 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -173,6 +173,9 @@ mymain(int argc, char **argv)
 DO_TEST("disk-drive-cache-v2-wt");
 DO_TEST("disk-drive-cache-v2-wb");
 DO_TEST("disk-drive-cache-v2-none");
+DO_TEST("disk-drive-network-nbd");
+DO_TEST("disk-drive-network-rbd");
+DO_TEST("disk-drive-network-sheepdog");
 DO_TEST("disk-usb");
 DO_TEST("graphics-vnc");

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args
new file mode 100644
index 000..75d74ab
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args
@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M 
pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait 
-no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive 
file=nbd:example.org:6000,if=virtio,format=raw -net none -serial none -parallel 
none -usb
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml
new file mode 100644
index 000..863165d
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml
@@ -0,0 +1,32 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219200
+  219200
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+  
+  
+  
+
+
+  
+  
+
+  
+  
+
+
+
+  
+
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
new file mode 100644
index 000..7e19beb
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test CEPH_ARGS=-m 
mon1.example.org:6321,mon2.example.org:6322,mon3.example.org:6322 /usr/bin/qemu 
-S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait 
-no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive 
file=rbd:pool/image,if=virtio,format=raw -net none -serial none -parallel none 
-usb
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
new file mode 100644
index 000..118d890
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
@@ -0,0 +1,34 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219200
+  219200
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+  
+  
+  
+
+
+  
+  
+
+
+
+  
+  
+
+
+
+  
+
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args
new file mode 100644
index 000..147378f
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args
@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M 
pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait 
-no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive 
file=sheepdog:example.org:6000:image,if=virtio,format=raw -net none -serial 
none -parallel none -usb
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml
new file mode 100644
index 000..5313d75
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-netw

[libvirt] [PATCHv2 2/2] openvz: avoid potential buffer overflow

2010-12-07 Thread Eric Blake
* src/openvz/openvz_conf.c (openvzLoadDomains): Replace unsafe
sscanf with safe direct parsing.
(openvzGetVEID): Avoid lost integer overflow detection.
(openvzAssignUUIDs): Likewise, and detect readdir failure.
---

v2: new patch; plugs a potential security hole, since
*scanf("%s",fixed_width_buffer) is exploitable, but the
exploit could only happen if /usr/sbin/vzlist is compromised.

 src/openvz/openvz_conf.c |   39 +--
 1 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 800c1f4..2d1c41a 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -435,7 +435,7 @@ openvzFreeDriver(struct openvz_driver *driver)

 int openvzLoadDomains(struct openvz_driver *driver) {
 int veid, ret;
-char status[16];
+char *status;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 virDomainObjPtr dom = NULL;
 char temp[50];
@@ -451,13 +451,17 @@ int openvzLoadDomains(struct openvz_driver *driver) {
 if (virCommandRun(cmd, NULL) < 0)
 goto cleanup;

-line = *outbuf ? outbuf : NULL;
-while (line) {
-if (sscanf(line, "%d %s\n", &veid, status) != 2) {
+line = outbuf;
+while (line[0] != '\0') {
+if (virStrToLong_i(line, &status, 10, &veid) < 0 ||
+*status++ != ' ' ||
+(line = strchr(status, '\n')) == NULL) {
 openvzError(VIR_ERR_INTERNAL_ERROR, "%s",
 _("Failed to parse vzlist output"));
 goto cleanup;
 }
+*line++ = '\0';

 if (VIR_ALLOC(dom) < 0)
 goto no_memory;
@@ -527,10 +531,6 @@ int openvzLoadDomains(struct openvz_driver *driver) {

 virDomainObjUnlock(dom);
 dom = NULL;
-
-line = strchr(line, '\n');
-if (line)
-line++;
 }

 virCommandFree(cmd);
@@ -953,8 +953,9 @@ static int openvzAssignUUIDs(void)
 DIR *dp;
 struct dirent *dent;
 char *conf_dir;
-int vpsid, res;
-char ext[8];
+int vpsid;
+char *ext;
+int ret = 0;

 conf_dir = openvzLocateConfDir();
 if (conf_dir == NULL)
@@ -966,16 +967,25 @@ static int openvzAssignUUIDs(void)
 return 0;
 }

+errno = 0;
 while ((dent = readdir(dp))) {
-res = sscanf(dent->d_name, "%d.%5s", &vpsid, ext);
-if (!(res == 2 && STREQ(ext, "conf")))
+if (virStrToLong_i(dent->d_name, &ext, 10, &vpsid) < 0 ||
+*ext++ != '.' ||
+STRNEQ(ext, "conf"))
 continue;
 if (vpsid > 0) /* '0.conf' belongs to the host, ignore it */
 openvzSetUUID(vpsid);
+errno = 0;
+}
+if (errno) {
+openvzError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Failed to scan configuration directory"));
+ret = -1;
 }
+
 closedir(dp);
 VIR_FREE(conf_dir);
-return 0;
+return ret;
 }


@@ -987,6 +997,7 @@ static int openvzAssignUUIDs(void)
 int openvzGetVEID(const char *name) {
 virCommandPtr cmd;
 char *outbuf;
+char *temp;
 int veid;
 bool ok;

@@ -999,7 +1010,7 @@ int openvzGetVEID(const char *name) {
 }

 virCommandFree(cmd);
-ok = sscanf(outbuf, "%d\n", &veid) == 1;
+ok = virStrToLong_i(outbuf, &temp, 10, &veid) == 0 && *temp == '\n';
 VIR_FREE(outbuf);

 if (ok && veid >= 0)
-- 
1.7.3.2

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


[libvirt] [PATCHv2 1/2] openvz: convert popen to virCommand

2010-12-07 Thread Eric Blake
popen must be matched with pclose (not fclose), or it will leak
resources.  Furthermore, it is a lousy interface when it comes to
signal handling.  We're much better off using our decent command
wrapper.  Note that virCommand guarantees that VIR_FREE(outbuf) is
both required and safe to call, whether virCommandRun succeeded or
failed.

* src/openvz/openvz_conf.c (openvzLoadDomains, openvzGetVEID):
Replace popen with virCommand usage.
---

v2: avoid memory leak on failure in openvzGetVEID

This patch is awaiting review on these prerequisites:
https://www.redhat.com/archives/libvir-list/2010-December/msg00318.html
https://www.redhat.com/archives/libvir-list/2010-December/msg00321.html

 src/openvz/openvz_conf.c |   55 -
 1 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 863af93..800c1f4 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -51,6 +51,7 @@
 #include "util.h"
 #include "nodeinfo.h"
 #include "files.h"
+#include "command.h"

 #define VIR_FROM_THIS VIR_FROM_OPENVZ

@@ -433,26 +434,26 @@ openvzFreeDriver(struct openvz_driver *driver)


 int openvzLoadDomains(struct openvz_driver *driver) {
-FILE *fp;
 int veid, ret;
 char status[16];
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 virDomainObjPtr dom = NULL;
 char temp[50];
+char *outbuf = NULL;
+char *line;
+virCommandPtr cmd = NULL;

 if (openvzAssignUUIDs() < 0)
 return -1;

-if ((fp = popen(VZLIST " -a -ovpsid,status -H 2>/dev/null", "r")) == NULL) 
{
-openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed"));
-return -1;
-}
-
-while (!feof(fp)) {
-if (fscanf(fp, "%d %s\n", &veid, status) != 2) {
-if (feof(fp))
-break;
+cmd = virCommandNewArgList(VZLIST, "-a", "-ovpsid,status", "-H", NULL);
+virCommandSetOutputBuffer(cmd, &outbuf);
+if (virCommandRun(cmd, NULL) < 0)
+goto cleanup;

+line = *outbuf ? outbuf : NULL;
+while (line) {
+if (sscanf(line, "%d %s\n", &veid, status) != 2) {
 openvzError(VIR_ERR_INTERNAL_ERROR, "%s",
 _("Failed to parse vzlist output"));
 goto cleanup;
@@ -526,9 +527,14 @@ int openvzLoadDomains(struct openvz_driver *driver) {

 virDomainObjUnlock(dom);
 dom = NULL;
+
+line = strchr(line, '\n');
+if (line)
+line++;
 }

-VIR_FORCE_FCLOSE(fp);
+virCommandFree(cmd);
+VIR_FREE(outbuf);

 return 0;

@@ -536,7 +542,8 @@ int openvzLoadDomains(struct openvz_driver *driver) {
 virReportOOMError();

  cleanup:
-VIR_FORCE_FCLOSE(fp);
+virCommandFree(cmd);
+VIR_FREE(outbuf);
 if (dom)
 virDomainObjUnref(dom);
 return -1;
@@ -978,27 +985,23 @@ static int openvzAssignUUIDs(void)
  */

 int openvzGetVEID(const char *name) {
-char *cmd;
+virCommandPtr cmd;
+char *outbuf;
 int veid;
-FILE *fp;
 bool ok;

-if (virAsprintf(&cmd, "%s %s -ovpsid -H", VZLIST, name) < 0) {
-openvzError(VIR_ERR_INTERNAL_ERROR, "%s",
-_("virAsprintf failed"));
+cmd = virCommandNewArgList(VZLIST, name, "-ovpsid", "-H", NULL);
+virCommandSetOutputBuffer(cmd, &outbuf);
+if (virCommandRun(cmd, NULL) < 0) {
+virCommandFree(cmd);
+VIR_FREE(outbuf);
 return -1;
 }

-fp = popen(cmd, "r");
-VIR_FREE(cmd);
-
-if (fp == NULL) {
-openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed"));
-return -1;
-}
+virCommandFree(cmd);
+ok = sscanf(outbuf, "%d\n", &veid) == 1;
+VIR_FREE(outbuf);

-ok = fscanf(fp, "%d\n", &veid ) == 1;
-VIR_FORCE_FCLOSE(fp);
 if (ok && veid >= 0)
 return veid;

-- 
1.7.3.2

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


[libvirt] [PATCHv2 2/2] sysinfo: convert to virCommand

2010-12-07 Thread Eric Blake
* src/util/sysinfo.c (virSysinfoRead): Use virCommand instead of
virExec.
---
v2: remove unused variable, rebase on top of whitespace cleanups.

This patch is awaiting review on these prerequisites:
https://www.redhat.com/archives/libvir-list/2010-December/msg00318.html
https://www.redhat.com/archives/libvir-list/2010-December/msg00321.html

 src/util/sysinfo.c |   45 ++---
 1 files changed, 6 insertions(+), 39 deletions(-)

diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index a02bd57..5b40a10 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -36,6 +36,7 @@
 #include "conf/domain_conf.h"
 #include "logging.h"
 #include "memory.h"
+#include "command.h"

 #define VIR_FROM_THIS VIR_FROM_SYSINFO

@@ -94,12 +95,9 @@ virSysinfoRead(void) {
 virSysinfoDefPtr
 virSysinfoRead(void) {
 char *path, *cur, *eol, *base;
-int pid, outfd = -1, errfd = -1;
 virSysinfoDefPtr ret = NULL;
-const char *argv[] = { NULL, "-q", "-t", "0,1", NULL };
-int res, waitret, exitstatus;
 char *outbuf = NULL;
-char *errbuf = NULL;
+virCommandPtr cmd;

 path = virFindFileInPath(SYSINFO_SMBIOS_DECODER);
 if (path == NULL) {
@@ -108,43 +106,13 @@ virSysinfoRead(void) {
  SYSINFO_SMBIOS_DECODER);
 return NULL;
 }
-argv[0] = path;

-res = virExec(argv, NULL, NULL, &pid, -1, &outfd, &errfd,
-  VIR_EXEC_NONE | VIR_EXEC_NONBLOCK);
-if (res < 0) {
+cmd = virCommandNewArgList(path, "-q", "-t", "0,1", NULL);
+virCommandSetOutputBuffer(cmd, &outbuf);
+if (virCommandRun(cmd, NULL) < 0) {
 virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
  _("Failed to execute command %s"),
  path);
-res = 1;
-goto cleanup;
-}
-
-/*
- * we are interested in the output, capture it and errors too
- */
-if (virPipeReadUntilEOF(outfd, errfd, &outbuf, &errbuf) < 0) {
-virReportSystemError(errno, _("cannot wait for '%s'"), path);
-while (waitpid(pid, &exitstatus, 0) == -1 && errno == EINTR)
-;
-goto cleanup;
-}
-
-if (outbuf)
-VIR_DEBUG("Command stdout: %s", outbuf);
-if (errbuf)
-VIR_DEBUG("Command stderr: %s", errbuf);
-
-while ((waitret = waitpid(pid, &exitstatus, 0) == -1) &&
-   (errno == EINTR));
-if (waitret == -1) {
-virReportSystemError(errno, _("Failed to wait for '%s'"), path);
-goto cleanup;
-}
-if (exitstatus != 0) {
-virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
- _("command %s failed with error code %d:%s"),
- path, exitstatus, errbuf);
 goto cleanup;
 }

@@ -227,8 +195,7 @@ virSysinfoRead(void) {

 cleanup:
 VIR_FREE(outbuf);
-VIR_FREE(errbuf);
-VIR_FREE(path);
+virCommandFree(cmd);

 return ret;

-- 
1.7.3.2

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


[libvirt] [PATCHv2 1/2] sysinfo: formatting cleanups

2010-12-07 Thread Eric Blake
* src/util/sysinfo.c: Indentation and () fixups.
---

No semantic change; noticed this while rebasing 5/5 of previous series
(1-4 of previous series are now pushed).

 src/util/sysinfo.c |   37 ++---
 1 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index cf41773..a02bd57 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -39,7 +39,7 @@

 #define VIR_FROM_THIS VIR_FROM_SYSINFO

-#define virSmbiosReportError(code, ...)  \
+#define virSmbiosReportError(code, ...)   \
 virReportErrorHelper(NULL, VIR_FROM_SYSINFO, code, __FILE__,  \
  __FUNCTION__, __LINE__, __VA_ARGS__)

@@ -88,7 +88,7 @@ virSysinfoRead(void) {
  */
 virReportSystemError(ENOSYS, "%s",
  _("Host sysinfo extraction not supported on this platform"));
-return(NULL);
+return NULL;
 }
 #else
 virSysinfoDefPtr
@@ -104,9 +104,9 @@ virSysinfoRead(void) {
 path = virFindFileInPath(SYSINFO_SMBIOS_DECODER);
 if (path == NULL) {
 virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Failed to find path for %s binary"),
+ _("Failed to find path for %s binary"),
  SYSINFO_SMBIOS_DECODER);
-return(NULL);
+return NULL;
 }
 argv[0] = path;

@@ -114,7 +114,7 @@ virSysinfoRead(void) {
   VIR_EXEC_NONE | VIR_EXEC_NONBLOCK);
 if (res < 0) {
 virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Failed to execute command %s"),
+ _("Failed to execute command %s"),
  path);
 res = 1;
 goto cleanup;
@@ -143,7 +143,7 @@ virSysinfoRead(void) {
 }
 if (exitstatus != 0) {
 virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
-   _("command %s failed with error code %d:%s"),
+ _("command %s failed with error code %d:%s"),
  path, exitstatus, errbuf);
 goto cleanup;
 }
@@ -159,25 +159,25 @@ virSysinfoRead(void) {
 cur += 8;
 eol = strchr(cur, '\n');
 if ((eol) && ((ret->bios_vendor = strndup(cur, eol - cur)) == NULL))
-goto no_memory;
+goto no_memory;
 }
 if ((cur = strstr(base, "Version: ")) != NULL) {
 cur += 9;
 eol = strchr(cur, '\n');
 if ((eol) && ((ret->bios_version = strndup(cur, eol - cur)) == NULL))
-goto no_memory;
+goto no_memory;
 }
 if ((cur = strstr(base, "Release Date: ")) != NULL) {
 cur += 14;
 eol = strchr(cur, '\n');
 if ((eol) && ((ret->bios_date = strndup(cur, eol - cur)) == NULL))
-goto no_memory;
+goto no_memory;
 }
 if ((cur = strstr(base, "BIOS Revision: ")) != NULL) {
 cur += 15;
 eol = strchr(cur, '\n');
 if ((eol) && ((ret->bios_release = strndup(cur, eol - cur)) == NULL))
-goto no_memory;
+goto no_memory;
 }
 if ((base = strstr(outbuf, "System Information")) == NULL)
 goto cleanup;
@@ -186,43 +186,43 @@ virSysinfoRead(void) {
 eol = strchr(cur, '\n');
 if ((eol) &&
 ((ret->system_manufacturer = strndup(cur, eol - cur)) == NULL))
-goto no_memory;
+goto no_memory;
 }
 if ((cur = strstr(base, "Product Name: ")) != NULL) {
 cur += 14;
 eol = strchr(cur, '\n');
 if ((eol) && ((ret->system_product = strndup(cur, eol - cur)) == NULL))
-goto no_memory;
+goto no_memory;
 }
 if ((cur = strstr(base, "Version: ")) != NULL) {
 cur += 9;
 eol = strchr(cur, '\n');
 if ((eol) && ((ret->system_version = strndup(cur, eol - cur)) == NULL))
-goto no_memory;
+goto no_memory;
 }
 if ((cur = strstr(base, "Serial Number: ")) != NULL) {
 cur += 15;
 eol = strchr(cur, '\n');
 if ((eol) && ((ret->system_serial = strndup(cur, eol - cur)) == NULL))
-goto no_memory;
+goto no_memory;
 }
 if ((cur = strstr(base, "UUID: ")) != NULL) {
 cur += 6;
 eol = strchr(cur, '\n');
 if ((eol) && ((ret->system_uuid = strndup(cur, eol - cur)) == NULL))
-goto no_memory;
+goto no_memory;
 }
 if ((cur = strstr(base, "SKU Number: ")) != NULL) {
 cur += 12;
 eol = strchr(cur, '\n');
 if ((eol) && ((ret->system_sku = strndup(cur, eol - cur)) == NULL))
-goto no_memory;
+goto no_memory;
 }
 if ((cur = strstr(base, "Family: ")) != NULL) {
 cur += 8;
 eol = strchr(cur, '\n');
 if ((eol) && ((ret->system_family = strndup(cur, eol 

Re: [libvirt] [PATCHv2 2/2] openvz: avoid potential buffer overflow

2010-12-07 Thread Matthias Bolte
2010/12/7 Eric Blake :
> * src/openvz/openvz_conf.c (openvzLoadDomains): Replace unsafe
> sscanf with safe direct parsing.
> (openvzGetVEID): Avoid lost integer overflow detection.
> (openvzAssignUUIDs): Likewise, and detect readdir failure.
> ---
>
> v2: new patch; plugs a potential security hole, since
> *scanf("%s",fixed_width_buffer) is exploitable, but the
> exploit could only happen if /usr/sbin/vzlist is compromised.
>
>  src/openvz/openvz_conf.c |   39 +--
>  1 files changed, 25 insertions(+), 14 deletions(-)
>

ACK.

Matthias

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

Re: [libvirt] [PATCH] command: enforce fd vs. buffer considerations

2010-12-07 Thread Matthias Bolte
2010/12/7 Eric Blake :
> * docs/internals/command.html.in: Better documentation of buffer
> vs. fd considerations.
> * src/util/command.c (virCommandRunAsync): Reject raw execution
> with string io.
> (virCommandRun): Reject execution with user-specified fds not
> visiting a regular file.
> ---
>
> Perhaps we need to relax the fstat check to permit block devices
> in addition to regular files; but that can be a later patch if needed.
>
>  docs/internals/command.html.in |   35 ++-
>  src/util/command.c             |   37 -
>  2 files changed, 62 insertions(+), 10 deletions(-)

> +    if (async_io ? (!(cmd->flags & VIR_EXEC_DAEMON) || string_io)
> +        : ((cmd->flags & VIR_EXEC_DAEMON) && string_io)) {
> +        virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                        _("invalid use of command API"));
> +        return -1;
> +    }

> +    /* Buffer management can only be requested via virCommandRun.  */
> +    if ((cmd->inbuf && cmd->infd == -1) ||
> +        (cmd->outbuf && cmd->outfdptr != &cmd->outfd) ||
> +        (cmd->errbuf && cmd->errfdptr != &cmd->errfd)) {
> +        virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                        _("invalid use of command API"));
> +        return -1;
> +    }

Maybe make the error message about invalid use distinct from each
other. That would allow to tell based on the error message what type
of invalid usage has happened.

ACK.

Matthias

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

Re: [libvirt] [PATCHv3] command: improve behavior on no output

2010-12-07 Thread Matthias Bolte
2010/12/7 Eric Blake :
> Guarantee that outbuf/errbuf are allocated on success, even if to the
> empty string.  Caller always has to free the result, and empty output
> check requires checking if *outbuf=='\0'.  Makes the API easier to use
> safely.  Failure is best effort allocation (some paths, like
> out-of-memory, cannot allocate a buffer, but most do), so caller must
> free buffer on failure; but this now guarantees that VIR_FREE(buf)
> will be safe.
>
> * docs/internals/command.html.in: Update documentation.
> * src/util/command.c (virCommandSetOutputBuffer)
> (virCommandSetErrorBuffer, virCommandProcessIO) Guarantee empty
> string on no output.
> * tests/commandtest.c (test17): New test.
> ---
>
> Everyone liked the safety of v2 over the speed of v1; and in looking
> at it more, I found even more ways to make it safer.  Hence this v3.
>
>  docs/internals/command.html.in |   12 ++--
>  src/util/command.c             |   53 
>  tests/commandtest.c            |   58 
> 
>  3 files changed, 108 insertions(+), 15 deletions(-)

> diff --git a/tests/commandtest.c b/tests/commandtest.c
> index b7261e9..20e56bc 100644
> --- a/tests/commandtest.c
> +++ b/tests/commandtest.c
> @@ -595,6 +595,63 @@ cleanup:
>     return ret;
>  }
>
> +/*
> + * Test string handling when no output is present.
> + */
> +static int test17(const void *unused ATTRIBUTE_UNUSED)
> +{
> +    virCommandPtr cmd = virCommandNew("/bin/true");
> +    int ret = -1;
> +    char *outbuf;
> +    char *errbuf;
> +
> +    virCommandSetOutputBuffer(cmd, &outbuf);
> +    if (outbuf != NULL) {
> +        puts("buffer not sanitized at registration");
> +        goto cleanup;
> +    }
> +
> +    if (virCommandRun(cmd, NULL) < 0) {
> +        virErrorPtr err = virGetLastError();
> +        printf("Cannot run child %s\n", err->message);
> +        goto cleanup;
> +    }
> +
> +    if (!outbuf || *outbuf) {
> +        puts("output string not allocated");

The error message doesn't fit to the *outbuf != \0 test.

> +        goto cleanup;
> +    }
> +    VIR_FREE(outbuf);
> +    if ((outbuf = strdup("should not be leaked")) == NULL) {
> +        puts("test framework failure");
> +        goto cleanup;
> +    }
> +
> +    virCommandSetErrorBuffer(cmd, &errbuf);
> +    if (errbuf != NULL) {
> +        puts("buffer not sanitized at registration");
> +        goto cleanup;
> +    }
> +
> +    if (virCommandRun(cmd, NULL) < 0) {
> +        virErrorPtr err = virGetLastError();
> +        printf("Cannot run child %s\n", err->message);
> +        goto cleanup;
> +    }
> +
> +    if (!outbuf || *outbuf || !errbuf || *errbuf) {
> +        puts("output strings not allocated");

The error message doesn't fit to the *{out|err}buf != \0 test.

Also what about the case when running /bin/true prints to {std|err}out
for some reason? :)

ACK.

Matthias

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

Re: [libvirt] [PATCHv2 1/2] openvz: convert popen to virCommand

2010-12-07 Thread Matthias Bolte
2010/12/7 Eric Blake :
> popen must be matched with pclose (not fclose), or it will leak
> resources.  Furthermore, it is a lousy interface when it comes to
> signal handling.  We're much better off using our decent command
> wrapper.  Note that virCommand guarantees that VIR_FREE(outbuf) is
> both required and safe to call, whether virCommandRun succeeded or
> failed.
>
> * src/openvz/openvz_conf.c (openvzLoadDomains, openvzGetVEID):
> Replace popen with virCommand usage.
> ---
>
> v2: avoid memory leak on failure in openvzGetVEID
>
> This patch is awaiting review on these prerequisites:
> https://www.redhat.com/archives/libvir-list/2010-December/msg00318.html
> https://www.redhat.com/archives/libvir-list/2010-December/msg00321.html
>
>  src/openvz/openvz_conf.c |   55 -
>  1 files changed, 29 insertions(+), 26 deletions(-)
>

ACK.

Matthias

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

Re: [libvirt] [PATCH] command: enforce fd vs. buffer considerations

2010-12-07 Thread Eric Blake
On 12/07/2010 02:59 PM, Matthias Bolte wrote:
> 2010/12/7 Eric Blake :
>> * docs/internals/command.html.in: Better documentation of buffer
>> vs. fd considerations.
>> * src/util/command.c (virCommandRunAsync): Reject raw execution
>> with string io.
>> (virCommandRun): Reject execution with user-specified fds not
>> visiting a regular file.
>> +if (async_io ? (!(cmd->flags & VIR_EXEC_DAEMON) || string_io)
>> +: ((cmd->flags & VIR_EXEC_DAEMON) && string_io)) {
>> +virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +_("invalid use of command API"));
>> +return -1;
>> +}
> 
>> +/* Buffer management can only be requested via virCommandRun.  */
>> +if ((cmd->inbuf && cmd->infd == -1) ||
>> +(cmd->outbuf && cmd->outfdptr != &cmd->outfd) ||
>> +(cmd->errbuf && cmd->errfdptr != &cmd->errfd)) {
>> +virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +_("invalid use of command API"));
>> +return -1;
>> +}
> 
> Maybe make the error message about invalid use distinct from each
> other. That would allow to tell based on the error message what type
> of invalid usage has happened.
> 
> ACK.

I'm squashing this in before pushing:

diff --git i/src/util/command.c w/src/util/command.c
index d1d8f6d..3dfd33d 100644
--- i/src/util/command.c
+++ w/src/util/command.c
@@ -902,11 +902,18 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
 (*cmd->errfdptr == -1 ||
  fstat(*cmd->errfdptr, &st) < 0 || !S_ISREG(st.st_mode)))
 async_io = true;
-if (async_io ? (!(cmd->flags & VIR_EXEC_DAEMON) || string_io)
-: ((cmd->flags & VIR_EXEC_DAEMON) && string_io)) {
-virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
-_("invalid use of command API"));
-return -1;
+if (async_io) {
+if (!(cmd->flags & VIR_EXEC_DAEMON) || string_io) {
+virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("cannot mix caller fds with blocking
execution"));
+return -1;
+}
+} else {
+if ((cmd->flags & VIR_EXEC_DAEMON) && string_io) {
+virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("cannot mix string I/O with daemon"));
+return -1;
+}
 }

 /* If we have an input buffer, we need
@@ -1032,7 +1039,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
 (cmd->outbuf && cmd->outfdptr != &cmd->outfd) ||
 (cmd->errbuf && cmd->errfdptr != &cmd->errfd)) {
 virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
-_("invalid use of command API"));
+_("cannot mix string I/O with asynchronous
command"));
 return -1;
 }


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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv3] command: improve behavior on no output

2010-12-07 Thread Eric Blake
On 12/07/2010 03:17 PM, Matthias Bolte wrote:
>> +if (!outbuf || *outbuf) {
>> +puts("output string not allocated");
> 
> The error message doesn't fit to the *outbuf != \0 test.

Fixed by squashing this in:

diff --git i/tests/commandtest.c w/tests/commandtest.c
index 9dea6f3..e956205 100644
--- i/tests/commandtest.c
+++ w/tests/commandtest.c
@@ -633,7 +633,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED)
 }

 if (!outbuf || *outbuf) {
-puts("output string not allocated");
+puts("output buffer is not an allocated empty string");
 goto cleanup;
 }
 VIR_FREE(outbuf);
@@ -655,7 +655,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED)
 }

 if (!outbuf || *outbuf || !errbuf || *errbuf) {
-puts("output strings not allocated");
+puts("output buffers are not allocated empty strings");
 goto cleanup;
 }


> 
> Also what about the case when running /bin/true prints to {std|err}out
> for some reason? :)

Then you deserve a 'make check' failure, to force you to figure out why
your system is hosed. :)

> 
> ACK.

Thanks for the review; pushing soon.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv2 2/2] openvz: avoid potential buffer overflow

2010-12-07 Thread Eric Blake
On 12/07/2010 02:49 PM, Matthias Bolte wrote:
> 2010/12/7 Eric Blake :
>> * src/openvz/openvz_conf.c (openvzLoadDomains): Replace unsafe
>> sscanf with safe direct parsing.
>> (openvzGetVEID): Avoid lost integer overflow detection.
>> (openvzAssignUUIDs): Likewise, and detect readdir failure.
>> ---
>>
>> v2: new patch; plugs a potential security hole, since
>> *scanf("%s",fixed_width_buffer) is exploitable, but the
>> exploit could only happen if /usr/sbin/vzlist is compromised.
>>
>>  src/openvz/openvz_conf.c |   39 +--
>>  1 files changed, 25 insertions(+), 14 deletions(-)
>>
> 
> ACK.

Thanks; I've pushed the series.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Moved the nodeinfo command to the 'host' help keyword group

2010-12-07 Thread Justin Clift
---
 tools/virsh.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 9ce3202..6c48a62 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -10062,7 +10062,6 @@ static const vshCmdDef nodedevCmds[] = {
 {"nodedev-list", cmdNodeListDevices, opts_node_list_devices, 
info_node_list_devices},
 {"nodedev-reattach", cmdNodeDeviceReAttach, opts_node_device_reattach, 
info_node_device_reattach},
 {"nodedev-reset", cmdNodeDeviceReset, opts_node_device_reset, 
info_node_device_reset},
-{"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo},
 {NULL, NULL, NULL, NULL}
 };
 
@@ -10127,6 +10126,7 @@ static const vshCmdDef hostAndHypervisorCmds[] = {
 {"connect", cmdConnect, opts_connect, info_connect},
 {"freecell", cmdFreecell, opts_freecell, info_freecell},
 {"hostname", cmdHostname, NULL, info_hostname},
+{"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo},
 {"qemu-monitor-command", cmdQemuMonitorCommand, opts_qemu_monitor_command, 
info_qemu_monitor_command},
 {"uri", cmdURI, NULL, info_uri},
 {NULL, NULL, NULL, NULL}
-- 
1.7.3.2

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


Re: [libvirt] [PATCH] Moved the nodeinfo command to the 'host' help keyword group

2010-12-07 Thread Eric Blake
On 12/07/2010 04:08 PM, Justin Clift wrote:
> ---
>  tools/virsh.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

ACK; nodedev is for managing devices attached to a node, but nodeinfo is
more generic than that.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Moved the nodeinfo command to the 'host' help keyword group

2010-12-07 Thread Justin Clift
On 08/12/2010, at 10:16 AM, Eric Blake wrote:
> On 12/07/2010 04:08 PM, Justin Clift wrote:
>> ---
>> tools/virsh.c |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> ACK; nodedev is for managing devices attached to a node, but nodeinfo is
> more generic than that.

Thanks Eric.  Pushed. :)

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


[libvirt] [PATCH] maint: update to latest gnulib

2010-12-07 Thread Eric Blake
* .gnulib: Update to latest, for at least a stdint.h fix
* src/storage/storage_driver.c (storageVolumeZeroSparseFile)
(storageWipeExtent): Use better type, although it still triggers
spurious -Wformat warning on MacOS's gcc.
---

* .gnulib 48b1a1a...6491120 (47):
  > unistr/u8-strcmp: Avoid collision with libc function on Solaris 11.
  > Update internal documentation.
  > Put more information about failed tests into the test return codes.
  > Update for Solaris 11 2010-11.
  > nproc: Relax license.
  > maint: restore executable bit
  > utimecmp: fine-grained src to nearby coarse-grained dest
  > strerror_r-posix: Fix autoconf test.
  > Tests for module 'getdomainname'.
  > getdomainname: Use the system function when possible.
  > sys_socket: Ensure ss_family field on AIX.
  > readline: Improve configure output.
  > *printf-posix: Detect a bug on Solaris 10/x86.
  > Fix link error when module libunistring-optional is in use.
  > regex: Mention link dependencies.
  > ftoastr: Fix compilation error on Solaris.
  > Oops, fix typo in last commit.
  > autoupdate
  > autoupdate
  > getloadavg: Update documentation.
  > sys_socket: Fix test whether the functions are declared.
  > getpass: Make sure to get system declaration on some platforms.
  > iconv-h: Fix test-iconv-h-c++ failure on Solaris 11 2010-11.
  > ftoastr: comment fix
  > stdint: port to GCC 4.3 + OSX + Octave
  > doc: Corrections regarding MacOS X 10.4 and 10.5.
  > autoupdate
  > Uninstall ".bin" files installed by relocwrapper.
  > Update for NetBSD 5.0.
  > Update for HP-UX 11.23 and HP-UX 11.31.
  > Update for MacOS X 10.5.
  > bootstrap: add bootstrap_sync option.
  > Oops, fix incomplete ChangeLog entry.
  > Ensure that  is included before __GLIBC__ is tested.
  > memmem: Fix autoconf test.
  > Port to uClibc.
  > nproc: Fix condition.
  > Fix a comment. * lib/vasnprintf.c (VASNPRINTF): Fix comment.
  > ftoastr: don't assume snprintf
  > test-rename.h: fix compilation failure
  > maint.mk: add a URL discussing the n...@acronym policy
  > ftoastr: depend on snprintf, improve comments
  > ftoastr: port to hosts lacking strtof and strtold
  > c-strtold: Avoid link error on AIX 7.
  > intprops: new macro INT_BITS_STRLEN_BOUND
  > ftoastr: new module, for lossless conversion of floats to short strings
  > autoupdate

 .gnulib  |2 +-
 src/storage/storage_driver.c |6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/.gnulib b/.gnulib
index 48b1a1a..6491120 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 48b1a1ae7d5b0f1494aabd4c8a02fbfcec531026
+Subproject commit 64911207854610668b480939469282fdaeb96f74
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 0f099f0..67d043b 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1575,7 +1575,7 @@ storageVolumeZeroSparseFile(virStorageVolDefPtr vol,
 virReportSystemError(errno,
  _("Failed to truncate volume with "
"path '%s' to %ju bytes"),
- vol->target.path, (intmax_t)size);
+ vol->target.path, (uintmax_t)size);
 }

 out:
@@ -1597,13 +1597,13 @@ storageWipeExtent(virStorageVolDefPtr vol,
 size_t write_size = 0;

 VIR_DEBUG("extent logical start: %ju len: %ju",
-  (intmax_t)extent_start, (intmax_t)extent_length);
+  (uintmax_t)extent_start, (uintmax_t)extent_length);

 if ((ret = lseek(fd, extent_start, SEEK_SET)) < 0) {
 virReportSystemError(errno,
  _("Failed to seek to position %ju in volume "
"with path '%s'"),
- (intmax_t)extent_start, vol->target.path);
+ (uintmax_t)extent_start, vol->target.path);
 goto out;
 }

-- 
1.7.3.2

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


[libvirt] [PATCH] configure: improve misleading libnl missing error message

2010-12-07 Thread Justin Clift
This fixes a misleading error message saying the libnl package
needs to be installed, when it's really the libnl-devel package
needing to be installed.
---
 configure.ac |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure.ac b/configure.ac
index d26bc68..dde2cde 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2215,7 +2215,7 @@ LIBNL_LIBS=""
 if test "$with_macvtap" = "yes"; then
 PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [
 ], [
-AC_MSG_ERROR([libnl >= $LIBNL_REQUIRED is required for macvtap 
support])
+AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for macvtap 
support])
 ])
 fi
 
-- 
1.7.3.2

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


Re: [libvirt] [PATCH] maint: update to latest gnulib

2010-12-07 Thread Justin Clift
On 08/12/2010, at 11:32 AM, Eric Blake wrote:
> * .gnulib: Update to latest, for at least a stdint.h fix

Thanks for spending the time on this Eric.

ACK to this.


> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 0f099f0..67d043b 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -1575,7 +1575,7 @@ storageVolumeZeroSparseFile(virStorageVolDefPtr vol,
> virReportSystemError(errno,
>  _("Failed to truncate volume with "
>"path '%s' to %ju bytes"),
> - vol->target.path, (intmax_t)size);
> + vol->target.path, (uintmax_t)size);
> }
> 
> out:
> @@ -1597,13 +1597,13 @@ storageWipeExtent(virStorageVolDefPtr vol,
> size_t write_size = 0;
> 
> VIR_DEBUG("extent logical start: %ju len: %ju",
> -  (intmax_t)extent_start, (intmax_t)extent_length);
> +  (uintmax_t)extent_start, (uintmax_t)extent_length);
> 
> if ((ret = lseek(fd, extent_start, SEEK_SET)) < 0) {
> virReportSystemError(errno,
>  _("Failed to seek to position %ju in volume "
>"with path '%s'"),
> - (intmax_t)extent_start, vol->target.path);
> + (uintmax_t)extent_start, vol->target.path);
> goto out;
> }

This seems safe too.

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


Re: [libvirt] [PATCH] configure: improve misleading libnl missing error message

2010-12-07 Thread Eric Blake
On 12/07/2010 05:40 PM, Justin Clift wrote:
> This fixes a misleading error message saying the libnl package
> needs to be installed, when it's really the libnl-devel package
> needing to be installed.
> ---
>  configure.ac |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index d26bc68..dde2cde 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2215,7 +2215,7 @@ LIBNL_LIBS=""
>  if test "$with_macvtap" = "yes"; then
>  PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [
>  ], [
> -AC_MSG_ERROR([libnl >= $LIBNL_REQUIRED is required for macvtap 
> support])
> +AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for macvtap 
> support])

ACK.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] maint: update to latest gnulib

2010-12-07 Thread Eric Blake
On 12/07/2010 06:01 PM, Justin Clift wrote:
> On 08/12/2010, at 11:32 AM, Eric Blake wrote:
>> * .gnulib: Update to latest, for at least a stdint.h fix
> 
> Thanks for spending the time on this Eric.
> 
> ACK to this.

Thanks; pushed.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] configure: improve misleading libnl missing error message

2010-12-07 Thread Justin Clift
On 08/12/2010, at 1:05 PM, Eric Blake wrote:
> On 12/07/2010 05:40 PM, Justin Clift wrote:
>> This fixes a misleading error message saying the libnl package
>> needs to be installed, when it's really the libnl-devel package
>> needing to be installed.
>> ---
>> configure.ac |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/configure.ac b/configure.ac
>> index d26bc68..dde2cde 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -2215,7 +2215,7 @@ LIBNL_LIBS=""
>> if test "$with_macvtap" = "yes"; then
>> PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [
>> ], [
>> -AC_MSG_ERROR([libnl >= $LIBNL_REQUIRED is required for macvtap 
>> support])
>> +AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for 
>> macvtap support])
> 
> ACK.

Thanks Eric, pushed. :)

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


[libvirt] [PATCH] virsh: fix a typo in the memtune help description

2010-12-07 Thread Justin Clift
---
 tools/virsh.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 6c48a62..4e37f2d 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2894,8 +2894,9 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
  * "memtune" command
  */
 static const vshCmdInfo info_memtune[] = {
-{"help", N_("Get/Set memory paramters")},
-{"desc", N_("Get/Set the current memory paramters for the guest domain.\n" 
\
+{"help", N_("Get or set memory parameters")},
+{"desc", N_("Get or set the current memory parameters for a guest" \
+" domain.\n" \
 "To get the memory parameters use following command: \n\n" 
\
 "virsh # memtune ")},
 {NULL, NULL}
-- 
1.7.3.2

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


Re: [libvirt] [PATCH] virsh: fix a typo in the memtune help description

2010-12-07 Thread Osier Yang

于 2010年12月08日 12:20, Justin Clift 写道:

---
  tools/virsh.c |5 +++--
  1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 6c48a62..4e37f2d 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2894,8 +2894,9 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
   * "memtune" command
   */
  static const vshCmdInfo info_memtune[] = {
-{"help", N_("Get/Set memory paramters")},
-{"desc", N_("Get/Set the current memory paramters for the guest domain.\n" 
\
+{"help", N_("Get or set memory parameters")},
+{"desc", N_("Get or set the current memory parameters for a guest" \
+" domain.\n" \
  "To get the memory parameters use following command: 
\n\n" \
  "virsh # memtune")},
  {NULL, NULL}

should we describe what's the diffrence with bunch of "dommemstat",
"setmem", and "setmaxmem"?

- Osier

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

[libvirt] [PATCH] Fix memory leak in virCommandRun()

2010-12-07 Thread Hu Tao
---
 src/util/command.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/util/command.c b/src/util/command.c
index c0520ec..473d1fc 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -956,6 +956,8 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
 cmd->errbuf = NULL;
 }
 
+VIR_FREE(outbuf);
+VIR_FREE(errbuf);
 return ret;
 }
 
-- 
1.7.3

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


[libvirt] [PATCH v6 1/4] threadpool impl

2010-12-07 Thread Hu Tao
* src/util/threadpool.c, src/util/threadpool.h: Thread pool
  implementation
* src/Makefile.am: Build thread pool
* src/libvirt_private.syms: Export public functions
---
 cfg.mk   |1 +
 src/Makefile.am  |1 +
 src/libvirt_private.syms |6 +
 src/util/threadpool.c|  231 ++
 src/util/threadpool.h|   48 ++
 5 files changed, 287 insertions(+), 0 deletions(-)
 create mode 100644 src/util/threadpool.c
 create mode 100644 src/util/threadpool.h

diff --git a/cfg.mk b/cfg.mk
index 29de9d9..bda8c57 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -128,6 +128,7 @@ useless_free_options =  \
   --name=virStoragePoolObjFree \
   --name=virStoragePoolSourceFree  \
   --name=virStorageVolDefFree  \
+  --name=virThreadPoolFree \
   --name=xmlFree   \
   --name=xmlXPathFreeContext   \
   --name=xmlXPathFreeObject
diff --git a/src/Makefile.am b/src/Makefile.am
index 0923d60..196d8af 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -74,6 +74,7 @@ UTIL_SOURCES =
\
util/threads.c util/threads.h   \
util/threads-pthread.h  \
util/threads-win32.h\
+   util/threadpool.c util/threadpool.h \
util/uuid.c util/uuid.h \
util/util.c util/util.h \
util/xml.c util/xml.h   \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e2def6c..a0a598b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -898,3 +898,9 @@ virXPathStringLimit;
 virXPathULong;
 virXPathULongHex;
 virXPathULongLong;
+
+
+# threadpool.h
+virThreadPoolNew;
+virThreadPoolFree;
+virThreadPoolSendJob;
diff --git a/src/util/threadpool.c b/src/util/threadpool.c
new file mode 100644
index 000..8217591
--- /dev/null
+++ b/src/util/threadpool.c
@@ -0,0 +1,231 @@
+/*
+ * threadpool.c: a generic thread pool implementation
+ *
+ * Copyright (C) 2010 Hu Tao
+ * Copyright (C) 2010 Daniel P. Berrange
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Authors:
+ * Hu Tao 
+ * Daniel P. Berrange 
+ */
+
+#include 
+
+#include "threadpool.h"
+#include "memory.h"
+#include "threads.h"
+#include "virterror_internal.h"
+#include "ignore-value.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+typedef struct _virThreadPoolJob virThreadPoolJob;
+typedef virThreadPoolJob *virThreadPoolJobPtr;
+
+struct _virThreadPoolJob {
+virThreadPoolJobPtr next;
+
+void *data;
+};
+
+typedef struct _virThreadPoolJobList virThreadPoolJobList;
+typedef virThreadPoolJobList *virThreadPoolJobListPtr;
+
+struct _virThreadPoolJobList {
+virThreadPoolJobPtr head;
+virThreadPoolJobPtr *tail;
+};
+
+
+struct _virThreadPool {
+bool quit;
+
+virThreadPoolJobFunc jobFunc;
+void *jobOpaque;
+virThreadPoolJobList jobList;
+
+virMutex mutex;
+virCond cond;
+virCond quit_cond;
+
+size_t maxWorkers;
+size_t freeWorkers;
+size_t nWorkers;
+virThreadPtr workers;
+};
+
+static void virThreadPoolWorker(void *opaque)
+{
+virThreadPoolPtr pool = opaque;
+
+virMutexLock(&pool->mutex);
+
+while (1) {
+while (!pool->quit &&
+   !pool->jobList.head) {
+pool->freeWorkers++;
+if (virCondWait(&pool->cond, &pool->mutex) < 0) {
+pool->freeWorkers--;
+goto out;
+}
+pool->freeWorkers--;
+}
+
+if (pool->quit)
+break;
+
+virThreadPoolJobPtr job = pool->jobList.head;
+pool->jobList.head = pool->jobList.head->next;
+job->next = NULL;
+if (pool->jobList.tail == &job->next)
+pool->jobList.tail = &pool->jobList.head;
+
+virMutexUnlock(&pool->mutex);
+(pool->jobFunc)(job->data, pool->jobOpaque);
+VIR_FREE(job);
+virMutexLock(&pool->mutex);
+}
+
+out:
+pool->nWorkers--;
+if (pool->nWorkers == 0)
+ 

[libvirt] [PATCH v6 2/4] Add a new function doCoreDump

2010-12-07 Thread Hu Tao
This patch prepares for the next patch.
---
 src/qemu/qemu_driver.c |  132 +++-
 1 files changed, 74 insertions(+), 58 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f5164e1..bf101fc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6007,6 +6007,78 @@ cleanup:
 return ret;
 }
 
+static int doCoreDump(struct qemud_driver *driver,
+  virDomainObjPtr vm,
+  const char *path,
+  enum qemud_save_formats compress)
+{
+int fd = -1;
+int ret = -1;
+qemuDomainObjPrivatePtr priv;
+
+priv = vm->privateData;
+
+/* Create an empty file with appropriate ownership.  */
+if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_("failed to create '%s'"), path);
+goto cleanup;
+}
+
+if (VIR_CLOSE(fd) < 0) {
+virReportSystemError(errno,
+ _("unable to save file %s"),
+ path);
+goto cleanup;
+}
+
+if (driver->securityDriver &&
+driver->securityDriver->domainSetSavedStateLabel &&
+
driver->securityDriver->domainSetSavedStateLabel(driver->securityDriver,
+ vm, path) == -1)
+goto cleanup;
+
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+if (compress == QEMUD_SAVE_FORMAT_RAW) {
+const char *args[] = {
+"cat",
+NULL,
+};
+ret = qemuMonitorMigrateToFile(priv->mon,
+   QEMU_MONITOR_MIGRATE_BACKGROUND,
+   args, path, 0);
+} else {
+const char *prog = qemudSaveCompressionTypeToString(compress);
+const char *args[] = {
+prog,
+"-c",
+NULL,
+};
+ret = qemuMonitorMigrateToFile(priv->mon,
+   QEMU_MONITOR_MIGRATE_BACKGROUND,
+   args, path, 0);
+}
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+if (ret < 0)
+goto cleanup;
+
+ret = qemuDomainWaitForMigrationComplete(driver, vm);
+
+if (ret < 0)
+goto cleanup;
+
+if (driver->securityDriver &&
+driver->securityDriver->domainRestoreSavedStateLabel &&
+
driver->securityDriver->domainRestoreSavedStateLabel(driver->securityDriver,
+ vm, path) == -1)
+goto cleanup;
+
+cleanup:
+if (ret != 0)
+unlink(path);
+return ret;
+}
+
 static enum qemud_save_formats
 getCompressionType(struct qemud_driver *driver)
 {
@@ -6041,13 +6113,10 @@ static int qemudDomainCoreDump(virDomainPtr dom,
 struct qemud_driver *driver = dom->conn->privateData;
 virDomainObjPtr vm;
 int resume = 0, paused = 0;
-int ret = -1, fd = -1;
+int ret = -1;
 virDomainEventPtr event = NULL;
-enum qemud_save_formats compress;
 qemuDomainObjPrivatePtr priv;
 
-compress = getCompressionType(driver);
-
 qemuDriverLock(driver);
 vm = virDomainFindByUUID(&driver->domains, dom->uuid);
 
@@ -6069,26 +6138,6 @@ static int qemudDomainCoreDump(virDomainPtr dom,
 goto endjob;
 }
 
-/* Create an empty file with appropriate ownership.  */
-if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-_("failed to create '%s'"), path);
-goto endjob;
-}
-
-if (VIR_CLOSE(fd) < 0) {
-virReportSystemError(errno,
- _("unable to save file %s"),
- path);
-goto endjob;
-}
-
-if (driver->securityDriver &&
-driver->securityDriver->domainSetSavedStateLabel &&
-
driver->securityDriver->domainSetSavedStateLabel(driver->securityDriver,
- vm, path) == -1)
-goto endjob;
-
 /* Migrate will always stop the VM, so the resume condition is
independent of whether the stop command is issued.  */
 resume = (vm->state == VIR_DOMAIN_RUNNING);
@@ -6112,43 +6161,12 @@ static int qemudDomainCoreDump(virDomainPtr dom,
 }
 }
 
-qemuDomainObjEnterMonitorWithDriver(driver, vm);
-if (compress == QEMUD_SAVE_FORMAT_RAW) {
-const char *args[] = {
-"cat",
-NULL,
-};
-ret = qemuMonitorMigrateToFile(priv->mon,
-   QEMU_MONITOR_MIGRATE_BACKGROUND,
-   args, path, 0);
-} else {
-const char *prog = qemudSaveCompressionTypeToString(compress);
-const char *args[] = {
-prog,
-"-c",
-NULL,
-};
-ret

[libvirt] [PATCH v6 0/4] Support of auto-dump on watchdog event in libvirtd

2010-12-07 Thread Hu Tao
This patch series adds a new watchdog action `dump' which lets libvirtd
can do auto-dump when receiving a watchdog event from qemu guest.

In order to make the function work, there must be a watchdog device
added to guest, and guest must have a watchdog daemon running, for 
example, /etc/init.d/watchdog start or auto-started on boot.

Changes:

v6:

- remove struct qemud_worker and qemud_server.job, qemud_server.nworkers,
  qemud_server.nactiveworkers, qemud_server.workers.

v5:

- qemu_driver is passed into threadpool as opaque parameter rather than
  visit the global qemu_driver in worker function
- same situation as above of server in libvirtd.c
- also list auto_dump_path in src/qemu/libvirtd_qemu.aug and 
  src/qemu/test_libvirtd_qemu.aug
- check return value of qemuDomainObjEndJob for safety

v4:

- updated threadpool to follow libvirt naming style, use appropriate
  internals APIs, and hide the struct definitions from the header
  (by Daniel)
- fix an error that qemuDomainObjBeginJobWithDriver() get lost in
  qemuDomainCoreDump()
- use thread pool in libvirtd (qemud worker)

v3:

- let default auto-dump dir be /var/lib/libvirt/qemu/dump


Hu Tao (4):
  threadpool impl
  Add a new function doCoreDump
  Add a watchdog action `dump'
  Using threadpool API to manage qemud worker

 cfg.mk  |1 +
 daemon/libvirtd.c   |  187 +---
 daemon/libvirtd.h   |   16 +---
 src/Makefile.am |1 +
 src/conf/domain_conf.c  |1 +
 src/conf/domain_conf.h  |1 +
 src/libvirt_private.syms|6 +
 src/qemu/libvirtd_qemu.aug  |1 +
 src/qemu/qemu.conf  |5 +
 src/qemu/qemu_conf.c|   16 +++-
 src/qemu/qemu_conf.h|5 +
 src/qemu/qemu_driver.c  |  228 --
 src/qemu/test_libvirtd_qemu.aug |2 +
 src/util/threadpool.c   |  231 +++
 src/util/threadpool.h   |   48 
 15 files changed, 517 insertions(+), 232 deletions(-)
 create mode 100644 src/util/threadpool.c
 create mode 100644 src/util/threadpool.h

-- 
1.7.3


-- 
Thanks,
Hu Tao

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


[libvirt] [PATCH v6 3/4] Add a watchdog action `dump'

2010-12-07 Thread Hu Tao
`dump' watchdog action lets libvirtd to dump the guest when receives a
watchdog event (which probably means a guest crash)

Currently only qemu is supported.
---
 src/conf/domain_conf.c  |1 +
 src/conf/domain_conf.h  |1 +
 src/qemu/libvirtd_qemu.aug  |1 +
 src/qemu/qemu.conf  |5 ++
 src/qemu/qemu_conf.c|   16 ++-
 src/qemu/qemu_conf.h|5 ++
 src/qemu/qemu_driver.c  |   96 +++
 src/qemu/test_libvirtd_qemu.aug |2 +
 8 files changed, 126 insertions(+), 1 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9c54a59..109f6fc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -244,6 +244,7 @@ VIR_ENUM_IMPL(virDomainWatchdogAction, 
VIR_DOMAIN_WATCHDOG_ACTION_LAST,
   "shutdown",
   "poweroff",
   "pause",
+  "dump",
   "none")
 
 VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 899b19f..7f50b79 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -462,6 +462,7 @@ enum virDomainWatchdogAction {
 VIR_DOMAIN_WATCHDOG_ACTION_SHUTDOWN,
 VIR_DOMAIN_WATCHDOG_ACTION_POWEROFF,
 VIR_DOMAIN_WATCHDOG_ACTION_PAUSE,
+VIR_DOMAIN_WATCHDOG_ACTION_DUMP,
 VIR_DOMAIN_WATCHDOG_ACTION_NONE,
 
 VIR_DOMAIN_WATCHDOG_ACTION_LAST
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 78852f3..2f37015 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -37,6 +37,7 @@ module Libvirtd_qemu =
  | str_array_entry "cgroup_device_acl"
  | str_entry "save_image_format"
  | str_entry "dump_image_format"
+ | str_entry "auto_dump_path"
  | str_entry "hugetlbfs_mount"
  | bool_entry "relaxed_acs_check"
  | bool_entry "vnc_allow_host_audio"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index f4f965e..ba41f80 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -191,6 +191,11 @@
 # save_image_format = "raw"
 # dump_image_format = "raw"
 
+# When a domain is configured to be auto-dumped when libvirtd receives a
+# watchdog event from qemu guest, libvirtd will save dump files in directory
+# specified by auto_dump_path. Default value is /var/lib/libvirt/qemu/dump
+#
+# auto_dump_path = "/var/lib/libvirt/qemu/dump"
 
 # If provided by the host and a hugetlbfs mount point is configured,
 # a guest may request huge page backing.  When this mount point is
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index d81e6cc..9c7dd49 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -386,6 +386,17 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
 }
 }
 
+p = virConfGetValue (conf, "auto_dump_path");
+CHECK_TYPE ("auto_dump_path", VIR_CONF_STRING);
+if (p && p->str) {
+VIR_FREE(driver->autoDumpPath);
+if (!(driver->autoDumpPath = strdup(p->str))) {
+virReportOOMError();
+virConfFree(conf);
+return -1;
+}
+}
+
  p = virConfGetValue (conf, "hugetlbfs_mount");
  CHECK_TYPE ("hugetlbfs_mount", VIR_CONF_STRING);
  if (p && p->str) {
@@ -5270,7 +5281,10 @@ qemudBuildCommandLine(virConnectPtr conn,
 virCommandAddArg(cmd, optstr);
 VIR_FREE(optstr);
 
-const char *action = 
virDomainWatchdogActionTypeToString(watchdog->action);
+int act = watchdog->action;
+if (act == VIR_DOMAIN_WATCHDOG_ACTION_DUMP)
+act = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE;
+const char *action = virDomainWatchdogActionTypeToString(act);
 if (!action) {
 qemuReportError(VIR_ERR_INTERNAL_ERROR,
 "%s", _("invalid watchdog action"));
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a1556cb..53f8185 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -43,6 +43,7 @@
 # include "bitmap.h"
 # include "macvtap.h"
 # include "command.h"
+# include "threadpool.h"
 
 # define qemudDebug(fmt, ...) do {} while(0)
 
@@ -109,6 +110,8 @@ enum qemud_cmd_flags {
 struct qemud_driver {
 virMutex lock;
 
+virThreadPoolPtr workerPool;
+
 int privileged;
 
 uid_t user;
@@ -176,6 +179,8 @@ struct qemud_driver {
 char *saveImageFormat;
 char *dumpImageFormat;
 
+char *autoDumpPath;
+
 pciDeviceList *activePciHostdevs;
 
 virBitmapPtr reservedVNCPorts;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bf101fc..1a01807 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -85,6 +85,7 @@
 #include "files.h"
 #include "fdstream.h"
 #include "configmake.h"
+#include "threadpool.h"
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -137,6 +138,14 @@ struct _qemuDomainObjPrivate {
 int persistentAddrs;
 };
 
+

[libvirt] [PATCH v6 4/4] Using threadpool API to manage qemud worker

2010-12-07 Thread Hu Tao
---
 daemon/libvirtd.c |  187 -
 daemon/libvirtd.h |   16 +
 2 files changed, 30 insertions(+), 173 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 791b3dc..229c0cc 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -67,6 +67,7 @@
 #include "stream.h"
 #include "hooks.h"
 #include "virtaudit.h"
+#include "threadpool.h"
 #ifdef HAVE_AVAHI
 # include "mdns.h"
 #endif
@@ -248,7 +249,6 @@ static void sig_handler(int sig, siginfo_t * siginfo,
 
 static void qemudDispatchClientEvent(int watch, int fd, int events, void 
*opaque);
 static void qemudDispatchServerEvent(int watch, int fd, int events, void 
*opaque);
-static int qemudStartWorker(struct qemud_server *server, struct qemud_worker 
*worker);
 
 void
 qemudClientMessageQueuePush(struct qemud_client_message **queue,
@@ -842,18 +842,10 @@ static struct qemud_server *qemudInitialize(void) {
 VIR_FREE(server);
 return NULL;
 }
-if (virCondInit(&server->job) < 0) {
-VIR_ERROR0(_("cannot initialize condition variable"));
-virMutexDestroy(&server->lock);
-VIR_FREE(server);
-return NULL;
-}
 
 if (virEventInit() < 0) {
 VIR_ERROR0(_("Failed to initialize event system"));
 virMutexDestroy(&server->lock);
-if (virCondDestroy(&server->job) < 0)
-{}
 VIR_FREE(server);
 return NULL;
 }
@@ -1458,19 +1450,6 @@ static int qemudDispatchServer(struct qemud_server 
*server, struct qemud_socket
 
 server->clients[server->nclients++] = client;
 
-if (server->nclients > server->nactiveworkers &&
-server->nactiveworkers < server->nworkers) {
-for (i = 0 ; i < server->nworkers ; i++) {
-if (!server->workers[i].hasThread) {
-if (qemudStartWorker(server, &server->workers[i]) < 0)
-return -1;
-server->nactiveworkers++;
-break;
-}
-}
-}
-
-
 return 0;
 
 error:
@@ -1534,100 +1513,28 @@ void qemudDispatchClientFailure(struct qemud_client 
*client) {
 VIR_FREE(client->addrstr);
 }
 
-
-/* Caller must hold server lock */
-static struct qemud_client *qemudPendingJob(struct qemud_server *server)
-{
-int i;
-for (i = 0 ; i < server->nclients ; i++) {
-virMutexLock(&server->clients[i]->lock);
-if (server->clients[i]->dx) {
-/* Delibrately don't unlock client - caller wants the lock */
-return server->clients[i];
-}
-virMutexUnlock(&server->clients[i]->lock);
-}
-return NULL;
-}
-
-static void *qemudWorker(void *data)
+static void qemudWorker(void *data, void *opaque)
 {
-struct qemud_worker *worker = data;
-struct qemud_server *server = worker->server;
-
-while (1) {
-struct qemud_client *client = NULL;
-struct qemud_client_message *msg;
-
-virMutexLock(&server->lock);
-while ((client = qemudPendingJob(server)) == NULL) {
-if (worker->quitRequest ||
-virCondWait(&server->job, &server->lock) < 0) {
-virMutexUnlock(&server->lock);
-return NULL;
-}
-}
-if (worker->quitRequest) {
-virMutexUnlock(&client->lock);
-virMutexUnlock(&server->lock);
-return NULL;
-}
-worker->processingCall = 1;
-virMutexUnlock(&server->lock);
-
-/* We own a locked client now... */
-client->refs++;
-
-/* Remove our message from dispatch queue while we use it */
-msg = qemudClientMessageQueueServe(&client->dx);
-
-/* This function drops the lock during dispatch,
- * and re-acquires it before returning */
-if (remoteDispatchClientRequest (server, client, msg) < 0) {
-VIR_FREE(msg);
-qemudDispatchClientFailure(client);
-client->refs--;
-virMutexUnlock(&client->lock);
-continue;
-}
-
-client->refs--;
-virMutexUnlock(&client->lock);
-
-virMutexLock(&server->lock);
-worker->processingCall = 0;
-virMutexUnlock(&server->lock);
-}
-}
-
-static int qemudStartWorker(struct qemud_server *server,
-struct qemud_worker *worker) {
-pthread_attr_t attr;
-pthread_attr_init(&attr);
-/* We want to join workers, so don't detach them */
-/*pthread_attr_setdetachstate(&attr, 1);*/
+struct qemud_server *server = opaque;
+struct qemud_client *client = data;
+struct qemud_client_message *msg;
 
-if (worker->hasThread)
-return -1;
+virMutexLock(&client->lock);
 
-worker->server = server;
-worker->hasThread = 1;
-worker->quitRequest = 0;
-worker->processingCall = 0;
+/* Remove our message from dispatch queue while we use it */
+msg = qemudClientMessageQueueServe(&client->dx);
 
-if (pthread_c

[libvirt] [C# bindings][PATCH]

2010-12-07 Thread arnaud.champion
?Hi,

here is another patch for C# bindings. I have update our project DAVIM with 
latest C# bindings and it show some naming errors. This patch correct it.

Arnaud

0001-Naming-update-in-Node-and-StoragePool-classes.patch
Description: Binary data
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] docs: updated virsh command reference download links to new version

2010-12-07 Thread Justin Clift
The new version lists every virsh command, its description, and
the version of libvirt where it became available.
---
 docs/virshcmdref.html.in |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/docs/virshcmdref.html.in b/docs/virshcmdref.html.in
index 0a19ca7..352c6b3 100644
--- a/docs/virshcmdref.html.in
+++ b/docs/virshcmdref.html.in
@@ -49,21 +49,21 @@
 
   
 Standard HTML format, multiple pages
-(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html-multi-page.tar.gz";>.tar.gz)
-(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html-multi-page.tar.bz2";>.tar.bz2)
-(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html-multi-page.zip";>.zip)
+(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html-multi-page.tar.gz";>.tar.gz)
+(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html-multi-page.tar.bz2";>.tar.bz2)
+(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html-multi-page.zip";>.zip)
   
   
 HTML format, one long page
-(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html-single.tar.gz";>.tar.gz)
-(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html-single.tar.bz2";>.tar.bz2)
-(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html-single.zip";>.zip)
+(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html-single.tar.gz";>.tar.gz)
+(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html-single.tar.bz2";>.tar.bz2)
+(http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html-single.zip";>.zip)
   
   
-http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0.pdf";>PDF
 format
+http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1.pdf";>PDF
 format
   
   
-http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0.epub";>ePub
 format
+http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1.epub";>ePub
 format
   
 
 
-- 
1.7.3.2

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


Re: [libvirt] [PATCH] docs: updated virsh command reference download links to new version

2010-12-07 Thread Justin Clift
On 08/12/2010, at 5:36 PM, Justin Clift wrote:
> The new version lists every virsh command, its description, and
> the version of libvirt where it became available.
> ---
> docs/virshcmdref.html.in |   16 
> 1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/virshcmdref.html.in b/docs/virshcmdref.html.in
> index 0a19ca7..352c6b3 100644
> --- a/docs/virshcmdref.html.in
> +++ b/docs/virshcmdref.html.in

Pushed this under the trivial rule, as it's just updating links
to point to the updated version of the Virsh Command Reference.

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