[libvirt] A error about live migration with --copy-storage-all

2010-07-30 Thread 姚远 
In fedora-13, libvirt is libvirt-0.8.2 and qemu is   qemu-kvm-0.12.3. In
virsh , the command of migrate --live fedora-13 qemu+ssh://
10.1.10.54/system is going well. but when I use the command of migrate
--live --copy-storage-all fedora-13 qemu+ssh://10.1.10.54/system, it show
the error:

internal errorqemu: could not open disk image /home/images/fedora-13.img: No
such file or directory
qemu: could not open disk image /home/images/fedora-13.img: No such file or
directory

10.1.10.54 is the ip of destination, and fedora-13 is my VM.
I don't know why. My VM fedora-13 can going well with virsh start
fedora-13 by /usr/bin/qemu-kvm. Who do live migration with
--copy-storage-all successfully can help me.

What is --copy-storage-inc  migration with non-shared storage with
incremental copy (same base image shared between source and destination)?
do that means I also need a shared storage?
The document of this I can find out is too less.
thank you!
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] openvzDomainCreateWithFlags: set domain id to the correct value

2010-07-30 Thread Jean-Baptiste Rouault 
I'm not sure if it's ok or not to modify dom-id as I did here.

Regards,
Jean-Baptiste
From 7fd011393a80da32949e4aa5468532140d250021 Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Rouault jean-baptiste.roua...@diateam.net
Date: Fri, 30 Jul 2010 10:36:06 +0200
Subject: [PATCH] openvzDomainCreateWithFlags: set domain id to the correct value

When an openvz domain is defined with virDomainDefineXML,
domain id is set to -1. A call to virDomainGetInfo after
starting the domain would then fail because this invalid
id is passed to openvzGetProcessInfo.
---
 src/openvz/openvz_driver.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 98381fb..e5bbdd0 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -992,6 +992,7 @@ openvzDomainCreateWithFlags(virDomainPtr dom, unsigned int flags)
 
 vm-pid = strtoI(vm-def-name);
 vm-def-id = vm-pid;
+dom-id = vm-pid;
 vm-state = VIR_DOMAIN_RUNNING;
 ret = 0;
 
-- 
1.7.0.4

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

Re: [libvirt] [PATCH] build: distribute libvirt_qemu.syms

2010-07-30 Thread Matthias Bolte 
2010/7/29 Eric Blake ebl...@redhat.com:
 * src/Makefile.am (EXTRA_DIST): Ensure 'make distcheck' and
 'rpmbuild' can reproduce a build.
 * daemon/Makefile.am (DAEMON_SOURCES): Likewise.
 ---

 The recent qemu monitor commands were incomplete.

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

 diff --git a/daemon/Makefile.am b/daemon/Makefile.am
 index 6769bab..963d64f 100644
 --- a/daemon/Makefile.am
 +++ b/daemon/Makefile.am
 @@ -10,6 +10,10 @@ DAEMON_SOURCES =                                     \
                remote_dispatch_table.h                 \
                remote_dispatch_args.h                  \
                remote_dispatch_ret.h                   \
 +               qemu_dispatch_prototypes.h              \
 +               qemu_dispatch_table.h                   \
 +               qemu_dispatch_args.h                    \
 +               qemu_dispatch_ret.h                     \
                ../src/remote/remote_protocol.c         \
                ../src/remote/qemu_protocol.c

 diff --git a/src/Makefile.am b/src/Makefile.am
 index 64d618b..a66eb2a 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -162,7 +162,8 @@ REMOTE_DRIVER_SOURCES =                                   
           \
                remote/qemu_protocol.c                          \
                remote/qemu_protocol.h

 -EXTRA_DIST += remote/remote_protocol.x remote/rpcgen_fix.pl
 +EXTRA_DIST += remote/remote_protocol.x remote/qemu_protocol.x \
 +               remote/rpcgen_fix.pl

  # Ensure that we don't change the struct or member names or member ordering
  # in remote_protocol.x  The embedded perl below needs a few comments, and
 @@ -1074,6 +1075,7 @@ libvirt_qemu_la_LDFLAGS = 
 $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_QEMU_SYMBOL_FILE) \
                          $(AM_LDFLAGS)
  libvirt_qemu_la_CFLAGS = $(AM_CFLAGS)
  libvirt_qemu_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD)
 +EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)

  libexec_PROGRAMS =

 --
 1.7.2


ACK.

Matthias

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

Re: [libvirt] [PATCH] build: distribute libvirt_qemu.syms

2010-07-30 Thread Eric Blake 
On 07/30/2010 06:27 AM, Matthias Bolte wrote:
 2010/7/29 Eric Blake ebl...@redhat.com:
 * src/Makefile.am (EXTRA_DIST): Ensure 'make distcheck' and
 'rpmbuild' can reproduce a build.
 * daemon/Makefile.am (DAEMON_SOURCES): Likewise.
 ---

 The recent qemu monitor commands were incomplete.

 
 ACK.

Thanks; applied.

-- 
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] esx: Switch from name to number checks in the subdrivers

2010-07-30 Thread Matthias Bolte 
2010/7/29 Daniel Veillard veill...@redhat.com:
 On Mon, Jul 26, 2010 at 03:12:50AM +0200, Matthias Bolte wrote:
 ---
  src/esx/esx_device_monitor.c   |    2 +-
  src/esx/esx_interface_driver.c |    2 +-
  src/esx/esx_network_driver.c   |    2 +-
  src/esx/esx_nwfilter_driver.c  |    2 +-
  src/esx/esx_secret_driver.c    |    2 +-
  src/esx/esx_storage_driver.c   |    2 +-
  6 files changed, 6 insertions(+), 6 deletions(-)



  Nicer, ACK,

 Daniel


Thanks, pushed.

Matthias

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

Re: [libvirt] [PATCH] esx: Improve blocked task detection and fix race condition

2010-07-30 Thread Matthias Bolte 
2010/7/29 Daniel Veillard veill...@redhat.com:
 On Mon, Jul 26, 2010 at 03:12:25AM +0200, Matthias Bolte wrote:
 esxVI_WaitForTaskCompletion can take a UUID to lookup the
 corresponding domain and check if the current task for it
 is blocked by a question. It calls another function to do
 this: esxVI_LookupAndHandleVirtualMachineQuestion looks up
 the VirtualMachine and checks for a question. If there is
 a question it calls esxVI_HandleVirtualMachineQuestion to
 handle it.

 If there was no question or it has been answered the call
 to esxVI_LookupAndHandleVirtualMachineQuestion returns 0.
 If any error occurred during the lookup and answering
 process -1 is returned. The problem with this is, that -1
 is also returned when there was no error but the question
 could not be answered. So esxVI_WaitForTaskCompletion cannot
 distinguish between this two situations and reports that a
 question is blocking the task even when there was actually
 another problem.

 This inherent problem didn't surface until vSphere 4.1 when
 you try to define a new domain. The driver tries to lookup
 the domain that is just in the process of being registered.
 There seems to be some kind of race condition and the driver
 manages to issue a lookup command before the ESX server was
 able to register the domain. This used to work before.

 Due to the return value problem described above the driver
 reported a false error message in that case.

 To solve this esxVI_WaitForTaskCompletion now takes an
 additional occurrence parameter that describes whether or
 not to expect the domain to be existent. Also add a new
 parameter to esxVI_LookupAndHandleVirtualMachineQuestion
 that allows to distinguish if the call returned -1 because
 of an actual error or because the question could not be
 answered.
 ---
  src/esx/esx_driver.c |   17 ++-
  src/esx/esx_vi.c     |   53 
 -
  src/esx/esx_vi.h     |    7 -
  src/esx/esx_vmx.c    |    4 +-
  4 files changed, 61 insertions(+), 20 deletions(-)

  Thanks for the detailed explanations !

  ACK,

 Daniel


Thanks, pushed.

Matthias

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

[libvirt] [PATCH] Fix the ACS checking in the PCI code.

2010-07-30 Thread Chris Lalancette 
When trying to assign a PCI device to a guest, we have
to check that all bridges upstream of that device support
ACS.  That means that we have to find the parent bridge of
the current device, check for ACS, then find the parent bridge
of that device, check for ACS, etc.  As it currently stands,
the code to do this iterates through all PCI devices on the
system, looking for a device that has a range of busses that
included the current device's bus.

That check is not restrictive enough, though.  Depending on
how we iterated through the list of PCI devices, we could first
find the *topmost* bridge in the system; since it necessarily had
a range of busses including the current device's bus, we
would only ever check the topmost bridge, and not check
any of the intermediate bridges.

Note that this also caused a fairly serious bug in the
secondary bus reset code, where we could erroneously
find and reset the topmost bus instead of the inner bus.

This patch changes pciGetParentDevice() so that it first
checks if a bridge device's secondary bus exactly matches
the bus of the device we are looking for.  If it does, we've
found the correct parent bridge and we are done.  If it does not,
then we check to see if this bridge device's busses *include* the
bus of the device we care about.  If so, we mark this bridge device
as best, and go on.  If we later find another bridge device whose
busses include this device, but is more restrictive, then we
free up the previous best and mark the new one as best.  This
algorithm ensures that in the normal case we find the direct
parent, but in the case that the parent bridge secondary bus
is not exactly the same as the device, we still find the
correct bridge.

This patch was tested by me on a 4-port NIC with a
bridge without ACS (where assignment failed), a 4-port
NIC with a bridge with ACS (where assignment succeeded),
and a 2-port NIC with no bridges (where assignment
succeeded).

Signed-off-by: Chris Lalancette clala...@redhat.com
---
 src/util/pci.c |   78 +++
 1 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/src/util/pci.c b/src/util/pci.c
index 26d55b8..f2890bd 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -283,6 +283,7 @@ pciIterDevices(pciIterPredicate predicate,
 DIR *dir;
 struct dirent *entry;
 int ret = 0;
+int rc;
 
 *matched = NULL;
 
@@ -322,11 +323,20 @@ pciIterDevices(pciIterPredicate predicate,
 break;
 }
 
-if (predicate(dev, check, data)) {
+rc = predicate(dev, check, data);
+if (rc  0) {
+/* the predicate returned an error, bail */
+pciFreeDevice(check);
+ret = -1;
+break;
+}
+else if (rc == 1) {
 VIR_DEBUG(%s %s: iter matched on %s, dev-id, dev-name, 
check-name);
 *matched = check;
+ret = 1;
 break;
 }
+
 pciFreeDevice(check);
 }
 closedir(dir);
@@ -510,10 +520,11 @@ pciBusContainsActiveDevices(pciDevice *dev,
 
 /* Is @check the parent of @dev ? */
 static int
-pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED)
+pciIsParent(pciDevice *dev, pciDevice *check, void *data)
 {
 uint16_t device_class;
 uint8_t header_type, secondary, subordinate;
+pciDevice **best = data;
 
 if (dev-domain != check-domain)
 return 0;
@@ -533,16 +544,54 @@ pciIsParent(pciDevice *dev, pciDevice *check, void *data 
ATTRIBUTE_UNUSED)
 
 VIR_DEBUG(%s %s: found parent device %s, dev-id, dev-name, 
check-name);
 
-/* No, it's superman! */
-return (dev-bus = secondary  dev-bus = subordinate);
+/* if the secondary bus exactly equals the device's bus, then we found
+ * the direct parent.  No further work is necessary
+ */
+if (dev-bus == secondary)
+return 1;
+
+/* otherwise, SRIOV allows VFs to be on different busses then their PFs.
+ * In this case, what we need to do is look for the best match; i.e.
+ * the most restrictive match that still satisfies all of the conditions.
+ */
+if (dev-bus  secondary  dev-bus = subordinate) {
+if (*best == NULL) {
+*best = pciGetDevice(check-domain, check-bus, check-slot,
+ check-function);
+if (*best == NULL)
+return -1;
+}
+else {
+/* OK, we had already recorded a previous best match for the
+ * parent.  See if the current device is more restrictive than the
+ * best, and if so, make it the new best
+ */
+if (secondary  pciRead8(*best, PCI_SECONDARY_BUS)) {
+pciFreeDevice(*best);
+*best = pciGetDevice(check-domain, check-bus, check-slot,
+ check-function);
+if (*best == NULL)
+return -1;
+}
+}
+}
+

[libvirt] [PATCH] Fix a memory leak in the qemudBuildCommandLine.

2010-07-30 Thread Chris Lalancette 
ADD_ARG_LIT should only be used for literal arguments,
since it duplicates the memory.  Since virBufferContentAndReset
is already allocating memory, we should only use ADD_ARG.

Signed-off-by: Chris Lalancette clala...@redhat.com
---
 src/qemu/qemu_conf.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 172986e..d0655fd 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -4110,7 +4110,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
 goto error;
 }
 
-ADD_ARG_LIT(virBufferContentAndReset(boot_buf));
+ADD_ARG(virBufferContentAndReset(boot_buf));
 }
 
 if (def-os.kernel) {
-- 
1.7.1.1

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

Re: [libvirt] [PATCH] Fix a memory leak in the qemudBuildCommandLine.

2010-07-30 Thread Eric Blake 
On 07/30/2010 07:47 AM, Chris Lalancette wrote:
 ADD_ARG_LIT should only be used for literal arguments,
 since it duplicates the memory.  Since virBufferContentAndReset
 is already allocating memory, we should only use ADD_ARG.

ACK.

Hmm, wondering if there is a way to make use of gcc's
__builtin_constant_p() to enforce that ADD_ARG_LIT is passed a string
constant.

-- 
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] Fix a memory leak in the qemudBuildCommandLine.

2010-07-30 Thread Eric Blake 
On 07/30/2010 08:02 AM, Chris Lalancette wrote:
 On 07/30/10 - 08:02:08AM, Eric Blake wrote:
 On 07/30/2010 07:47 AM, Chris Lalancette wrote:
 ADD_ARG_LIT should only be used for literal arguments,
 since it duplicates the memory.  Since virBufferContentAndReset
 is already allocating memory, we should only use ADD_ARG.

 ACK.

 Hmm, wondering if there is a way to make use of gcc's
 __builtin_constant_p() to enforce that ADD_ARG_LIT is passed a string
 constant.
 
 Interesting idea.  I know danpb wanted to remove all of the macros used in
 qemudBuildCommandLine, though, so you may want to consult with him before
 spending too much time on it.

True - his pending exec library rewrite will override anything we do to
the macros.

-- 
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] build: distribute libvirt_qemu.syms

2010-07-30 Thread Justin Clift 

On 07/30/2010 10:35 PM, Eric Blake wrote:

On 07/30/2010 06:27 AM, Matthias Bolte wrote:

2010/7/29 Eric Blakeebl...@redhat.com:

* src/Makefile.am (EXTRA_DIST): Ensure 'make distcheck' and
'rpmbuild' can reproduce a build.
* daemon/Makefile.am (DAEMON_SOURCES): Likewise.
---

The recent qemu monitor commands were incomplete.



ACK.


Thanks; applied.


Working fine here too. :)

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

Re: [libvirt] [PATCH] Fix a memory leak in the qemudBuildCommandLine.

2010-07-30 Thread Chris Lalancette 
On 07/30/10 - 08:02:08AM, Eric Blake wrote:
 On 07/30/2010 07:47 AM, Chris Lalancette wrote:
  ADD_ARG_LIT should only be used for literal arguments,
  since it duplicates the memory.  Since virBufferContentAndReset
  is already allocating memory, we should only use ADD_ARG.
 
 ACK.
 
 Hmm, wondering if there is a way to make use of gcc's
 __builtin_constant_p() to enforce that ADD_ARG_LIT is passed a string
 constant.

Interesting idea.  I know danpb wanted to remove all of the macros used in
qemudBuildCommandLine, though, so you may want to consult with him before
spending too much time on it.

--
Chris Lalancette

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

[libvirt] [PATCH] Fix DMI uuid parsing.

2010-07-30 Thread Chris Lalancette 
valgrind was complaining that virUUIDParse was depending on
an uninitialized value.  Indeed it was; virSetHostUUIDStr()
didn't initialize the dmiuuid buffer to 0's, meaning that
anything after the string read from /sys was uninitialized.
Clear out the dmiuuid buffer before use, and make sure to
always leave a \0 at the end.

Signed-off-by: Chris Lalancette clala...@redhat.com
---
 src/util/uuid.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/util/uuid.c b/src/util/uuid.c
index f188148..9cafc2a 100644
--- a/src/util/uuid.c
+++ b/src/util/uuid.c
@@ -286,7 +286,8 @@ virSetHostUUIDStr(const char *uuid)
 return EEXIST;
 
 if (!uuid) {
-if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid))) {
+memset(dmiuuid, 0, sizeof(dmiuuid));
+if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid) - 1)) {
 if (!virUUIDParse(dmiuuid, host_uuid))
 return 0;
 }
-- 
1.7.1.1

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

Re: [libvirt] [PATCH] Fix DMI uuid parsing.

2010-07-30 Thread Daniel Veillard 
On Fri, Jul 30, 2010 at 10:22:20AM -0400, Chris Lalancette wrote:
 valgrind was complaining that virUUIDParse was depending on
 an uninitialized value.  Indeed it was; virSetHostUUIDStr()
 didn't initialize the dmiuuid buffer to 0's, meaning that
 anything after the string read from /sys was uninitialized.
 Clear out the dmiuuid buffer before use, and make sure to
 always leave a \0 at the end.
 
 Signed-off-by: Chris Lalancette clala...@redhat.com
 ---
  src/util/uuid.c |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
 
 diff --git a/src/util/uuid.c b/src/util/uuid.c
 index f188148..9cafc2a 100644
 --- a/src/util/uuid.c
 +++ b/src/util/uuid.c
 @@ -286,7 +286,8 @@ virSetHostUUIDStr(const char *uuid)
  return EEXIST;
  
  if (!uuid) {
 -if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid))) {
 +memset(dmiuuid, 0, sizeof(dmiuuid));
 +if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid) - 1)) {
  if (!virUUIDParse(dmiuuid, host_uuid))
  return 0;
  }

  ACK,

Daniel

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

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

Re: [libvirt] [PATCH] Do not activate boot=on on devices when not using KVM

2010-07-30 Thread Daniel Veillard 
On Thu, Jul 29, 2010 at 09:36:28AM -0600, Eric Blake wrote:
 On 07/29/2010 09:27 AM, Daniel Veillard wrote:
  
Basically the 'boot=on' boot selection device is something present in
  KVM but not in upstream QEmu, as a result if we boot a QEmu domain
  without KVM acceleration we must disable boot=on ... even if the front
  end kvm binary expose that capability in the help page.
 
 ACK.

  thanks ! Pushed,

Daniel

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

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

[libvirt] [PATCH] qemu: Fix PCI address allocation

2010-07-30 Thread Jiri Denemark 
When attaching a PCI device which doesn't explicitly set its PCI
address, libvirt allocates the address automatically. The problem is
that when checking which PCI address is unused, we only check for those
with slot number higher than the highest slot number ever used.

Thus attaching/detaching such device several times in a row (31 is the
theoretical limit, less then 30 tries are enough in practise) makes any
further device attachment fail. Furthermore, attaching a device with
predefined PCI address to 0:0:31 immediately forbids attachment of any
PCI device without explicit address.

This patch changes the logic so that we always check all PCI addresses
before we say there is no PCI address available.
---
 src/qemu/qemu_conf.c |   14 --
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 57bc02f..eaebcc1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2055,8 +2055,6 @@ qemuAssignDeviceAliases(virDomainDefPtr def, unsigned 
long long qemuCmdFlags)
 #define QEMU_PCI_ADDRESS_LAST_SLOT 31
 struct _qemuDomainPCIAddressSet {
 virHashTablePtr used;
-int nextslot;
-/* XXX add domain, bus later when QEMU allows  1 */
 };
 
 
@@ -2148,9 +2146,6 @@ int 
qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
 return -1;
 }
 
-if (dev-addr.pci.slot  addrs-nextslot)
-addrs-nextslot = dev-addr.pci.slot + 1;
-
 return 0;
 }
 
@@ -2217,7 +2212,7 @@ int 
qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
 {
 int i;
 
-for (i = addrs-nextslot ; i = QEMU_PCI_ADDRESS_LAST_SLOT ; i++) {
+for (i = 0 ; i = QEMU_PCI_ADDRESS_LAST_SLOT ; i++) {
 virDomainDeviceInfo maybe;
 char *addr;
 
@@ -2228,13 +2223,14 @@ int 
qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
 
 addr = qemuPCIAddressAsString(maybe);
 
-VIR_DEBUG(Allocating PCI addr %s, addr);
-
 if (virHashLookup(addrs-used, addr)) {
+VIR_DEBUG(PCI addr %s already in use, addr);
 VIR_FREE(addr);
 continue;
 }
 
+VIR_DEBUG(Allocating PCI addr %s, addr);
+
 if (virHashAddEntry(addrs-used, addr, addr)  0) {
 VIR_FREE(addr);
 return -1;
@@ -2245,8 +2241,6 @@ int 
qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
 dev-addr.pci.bus = 0;
 dev-addr.pci.slot = i;
 
-addrs-nextslot = i + 1;
-
 return 0;
 }
 
-- 
1.7.2

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

Re: [libvirt] [PATCH] Fix DMI uuid parsing.

2010-07-30 Thread Chris Lalancette 
On 07/30/10 - 04:35:27PM, Daniel Veillard wrote:
 On Fri, Jul 30, 2010 at 10:22:20AM -0400, Chris Lalancette wrote:
  valgrind was complaining that virUUIDParse was depending on
  an uninitialized value.  Indeed it was; virSetHostUUIDStr()
  didn't initialize the dmiuuid buffer to 0's, meaning that
  anything after the string read from /sys was uninitialized.
  Clear out the dmiuuid buffer before use, and make sure to
  always leave a \0 at the end.
  
  Signed-off-by: Chris Lalancette clala...@redhat.com
  ---
   src/util/uuid.c |3 ++-
   1 files changed, 2 insertions(+), 1 deletions(-)
  
  diff --git a/src/util/uuid.c b/src/util/uuid.c
  index f188148..9cafc2a 100644
  --- a/src/util/uuid.c
  +++ b/src/util/uuid.c
  @@ -286,7 +286,8 @@ virSetHostUUIDStr(const char *uuid)
   return EEXIST;
   
   if (!uuid) {
  -if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid))) {
  +memset(dmiuuid, 0, sizeof(dmiuuid));
  +if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid) - 1)) {
   if (!virUUIDParse(dmiuuid, host_uuid))
   return 0;
   }
 
   ACK,

Thanks, pushed.

-- 
Chris Lalancette

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

Re: [libvirt] [PATCH] qemu: Fix PCI address allocation

2010-07-30 Thread Eric Blake 
On 07/30/2010 08:56 AM, Jiri Denemark wrote:
 When attaching a PCI device which doesn't explicitly set its PCI
 address, libvirt allocates the address automatically. The problem is
 that when checking which PCI address is unused, we only check for those
 with slot number higher than the highest slot number ever used.
 
 Thus attaching/detaching such device several times in a row (31 is the
 theoretical limit, less then 30 tries are enough in practise) makes any
 further device attachment fail. Furthermore, attaching a device with
 predefined PCI address to 0:0:31 immediately forbids attachment of any
 PCI device without explicit address.
 
 This patch changes the logic so that we always check all PCI addresses
 before we say there is no PCI address available.
 ---
  src/qemu/qemu_conf.c |   14 --
  1 files changed, 4 insertions(+), 10 deletions(-)
 
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index 57bc02f..eaebcc1 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -2055,8 +2055,6 @@ qemuAssignDeviceAliases(virDomainDefPtr def, unsigned 
 long long qemuCmdFlags)
  #define QEMU_PCI_ADDRESS_LAST_SLOT 31
  struct _qemuDomainPCIAddressSet {
  virHashTablePtr used;
 -int nextslot;
 -/* XXX add domain, bus later when QEMU allows  1 */

 -for (i = addrs-nextslot ; i = QEMU_PCI_ADDRESS_LAST_SLOT ; i++) {
 +for (i = 0 ; i = QEMU_PCI_ADDRESS_LAST_SLOT ; i++) {

Don't we still want to start from the last slot, and wrap around the
search only when we reach the end, rather than always starting from 0?
I'm thinking particularly about compatibility with older qemu, where
always starting from 0 risks interleaving new assignments among the
pre-assigned slots, and may end up allocating slots in a way that throws
off Windows.  Or am I worried about a non-issue?

-- 
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: Fix PCI address allocation

2010-07-30 Thread Chris Lalancette 
On 07/30/10 - 04:56:47PM, Jiri Denemark wrote:
 When attaching a PCI device which doesn't explicitly set its PCI
 address, libvirt allocates the address automatically. The problem is
 that when checking which PCI address is unused, we only check for those
 with slot number higher than the highest slot number ever used.
 
 Thus attaching/detaching such device several times in a row (31 is the
 theoretical limit, less then 30 tries are enough in practise) makes any
 further device attachment fail. Furthermore, attaching a device with
 predefined PCI address to 0:0:31 immediately forbids attachment of any
 PCI device without explicit address.
 
 This patch changes the logic so that we always check all PCI addresses
 before we say there is no PCI address available.

Yes, makes perfect sense.  Most of the patch is adding VIR_DEBUG() lines
and removing the nextslot; the real change is this line:

 @@ -2217,7 +2212,7 @@ int 
 qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
  {
  int i;
  
 -for (i = addrs-nextslot ; i = QEMU_PCI_ADDRESS_LAST_SLOT ; i++) {
 +for (i = 0 ; i = QEMU_PCI_ADDRESS_LAST_SLOT ; i++) {
  virDomainDeviceInfo maybe;
  char *addr;
  

ACK

-- 
Chris Lalancette

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

[libvirt] [PATCH] xenapi: Update ID after starting a domain

2010-07-30 Thread Matthias Bolte 
---
 src/xenapi/xenapi_driver.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index e385648..2262cef 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -1459,6 +1459,7 @@ xenapiDomainCreateWithFlags (virDomainPtr dom, unsigned 
int flags)
 xen_vm_set *vms;
 xen_vm vm;
 xen_session *session = ((struct _xenapiPrivate 
*)(dom-conn-privateData))-session;
+int64_t domid = -1;
 
 virCheckFlags(0, -1);
 
@@ -1475,6 +1476,10 @@ xenapiDomainCreateWithFlags (virDomainPtr dom, unsigned 
int flags)
 xen_vm_set_free(vms);
 return -1;
 }
+
+xen_vm_get_domid(session, domid, vm);
+dom-id = domid;
+
 xen_vm_set_free(vms);
 } else {
 if (vms) xen_vm_set_free(vms);
-- 
1.7.0.4

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

Re: [libvirt] [PATCH] xenapi: Update ID after starting a domain

2010-07-30 Thread Eric Blake 
On 07/30/2010 09:24 AM, Matthias Bolte wrote:
 ---
  src/xenapi/xenapi_driver.c |5 +
  1 files changed, 5 insertions(+), 0 deletions(-)
 
 diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
 index e385648..2262cef 100644
 --- a/src/xenapi/xenapi_driver.c
 +++ b/src/xenapi/xenapi_driver.c
 @@ -1459,6 +1459,7 @@ xenapiDomainCreateWithFlags (virDomainPtr dom, unsigned 
 int flags)
  xen_vm_set *vms;
  xen_vm vm;
  xen_session *session = ((struct _xenapiPrivate 
 *)(dom-conn-privateData))-session;
 +int64_t domid = -1;
  
  virCheckFlags(0, -1);
  
 @@ -1475,6 +1476,10 @@ xenapiDomainCreateWithFlags (virDomainPtr dom, 
 unsigned int flags)
  xen_vm_set_free(vms);
  return -1;
  }
 +
 +xen_vm_get_domid(session, domid, vm);
 +dom-id = domid;
 +

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] esx: Update ID after starting a domain

2010-07-30 Thread Matthias Bolte 
2010/7/30 Eric Blake ebl...@redhat.com:
 On 07/30/2010 09:23 AM, Matthias Bolte wrote:
 ---
  src/esx/esx_driver.c |    6 --
  1 files changed, 4 insertions(+), 2 deletions(-)

 diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
 index eb64556..964a3a5 100644
 --- a/src/esx/esx_driver.c
 +++ b/src/esx/esx_driver.c
 @@ -2483,6 +2483,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned 
 int flags)
      esxVI_ObjectContent *virtualMachine = NULL;
      esxVI_String *propertyNameList = NULL;
      esxVI_VirtualMachinePowerState powerState;
 +    int id = -1;
      esxVI_ManagedObjectReference *task = NULL;
      esxVI_TaskInfoState taskInfoState;

 @@ -2497,8 +2498,8 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned 
 int flags)
          esxVI_LookupVirtualMachineByUuidAndPrepareForTask
            (priv-primary, domain-uuid, propertyNameList, virtualMachine,
             priv-autoAnswer)  0 ||
 -        esxVI_GetVirtualMachinePowerState(virtualMachine,
 -                                          powerState)  0) {
 +        esxVI_GetVirtualMachinePowerState(virtualMachine, powerState)  0 
 ||
 +        esxVI_GetVirtualMachineIdentity(virtualMachine, id, NULL, NULL)  
 0) {

 ACK.  This leaves domain-id unchanged on failure, but I think that's
 okay, because the caller shouldn't be relying on the contents of domain
 on failure.


Thanks, pushed.

Matthias

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

Re: [libvirt] [PATCH] xenapi: Update ID after starting a domain

2010-07-30 Thread Matthias Bolte 
2010/7/30 Eric Blake ebl...@redhat.com:
 On 07/30/2010 09:24 AM, Matthias Bolte wrote:
 ---
  src/xenapi/xenapi_driver.c |    5 +
  1 files changed, 5 insertions(+), 0 deletions(-)

 diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
 index e385648..2262cef 100644
 --- a/src/xenapi/xenapi_driver.c
 +++ b/src/xenapi/xenapi_driver.c
 @@ -1459,6 +1459,7 @@ xenapiDomainCreateWithFlags (virDomainPtr dom, 
 unsigned int flags)
      xen_vm_set *vms;
      xen_vm vm;
      xen_session *session = ((struct _xenapiPrivate 
 *)(dom-conn-privateData))-session;
 +    int64_t domid = -1;

      virCheckFlags(0, -1);

 @@ -1475,6 +1476,10 @@ xenapiDomainCreateWithFlags (virDomainPtr dom, 
 unsigned int flags)
              xen_vm_set_free(vms);
              return -1;
          }
 +
 +        xen_vm_get_domid(session, domid, vm);
 +        dom-id = domid;
 +

 ACK.


Thanks, pushed.

Matthias

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

[libvirt] [PATCH] OpenVZ: implement suspend/resume driver APIs

2010-07-30 Thread Jean-Baptiste Rouault 
---
 src/openvz/openvz_driver.c |   84 ++-
 1 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index e5bbdd0..bdc0e92 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -503,6 +503,86 @@ static void openvzSetProgramSentinal(const char **prog, 
const char *key)
 }
 }
 
+static int openvzDomainSuspend(virDomainPtr dom) {
+struct openvz_driver *driver = dom-conn-privateData;
+virDomainObjPtr vm;
+const char *prog[] = {VZCTL, --quiet, chkpnt, PROGRAM_SENTINAL, 
--suspend, NULL};
+int ret = -1;
+
+openvzDriverLock(driver);
+vm = virDomainFindByUUID(driver-domains, dom-uuid);
+openvzDriverUnlock(driver);
+
+if (!vm) {
+openvzError(VIR_ERR_INVALID_DOMAIN, %s,
+_(no domain with matching uuid));
+goto cleanup;
+}
+
+if (!virDomainObjIsActive(vm)) {
+openvzError(VIR_ERR_OPERATION_INVALID, %s,
+_(Domain is not running));
+goto cleanup;
+}
+
+if (vm-state != VIR_DOMAIN_PAUSED) {
+openvzSetProgramSentinal(prog, vm-def-name);
+if (virRun(prog, NULL)  0) {
+openvzError(VIR_ERR_OPERATION_FAILED, %s,
+_(Suspend operation failed));
+goto cleanup;
+}
+vm-state = VIR_DOMAIN_PAUSED;
+}
+
+ret = 0;
+
+cleanup:
+if (vm)
+virDomainObjUnlock(vm);
+return ret;
+}
+
+static int openvzDomainResume(virDomainPtr dom) {
+  struct openvz_driver *driver = dom-conn-privateData;
+  virDomainObjPtr vm;
+  const char *prog[] = {VZCTL, --quiet, chkpnt, PROGRAM_SENTINAL, 
--resume, NULL};
+  int ret = -1;
+
+  openvzDriverLock(driver);
+  vm = virDomainFindByUUID(driver-domains, dom-uuid);
+  openvzDriverUnlock(driver);
+
+  if (!vm) {
+  openvzError(VIR_ERR_INVALID_DOMAIN, %s,
+  _(no domain with matching uuid));
+  goto cleanup;
+  }
+
+  if (!virDomainObjIsActive(vm)) {
+  openvzError(VIR_ERR_OPERATION_INVALID, %s,
+  _(Domain is not running));
+  goto cleanup;
+  }
+
+  if (vm-state == VIR_DOMAIN_PAUSED) {
+  openvzSetProgramSentinal(prog, vm-def-name);
+  if (virRun(prog, NULL)  0) {
+  openvzError(VIR_ERR_OPERATION_FAILED, %s,
+  _(Resume operation failed));
+  goto cleanup;
+  }
+  vm-state = VIR_DOMAIN_RUNNING;
+  }
+
+  ret = 0;
+
+cleanup:
+  if (vm)
+  virDomainObjUnlock(vm);
+  return ret;
+}
+
 static int openvzDomainShutdown(virDomainPtr dom) {
 struct openvz_driver *driver = dom-conn-privateData;
 virDomainObjPtr vm;
@@ -1491,8 +1571,8 @@ static virDriver openvzDriver = {
 openvzDomainLookupByID, /* domainLookupByID */
 openvzDomainLookupByUUID, /* domainLookupByUUID */
 openvzDomainLookupByName, /* domainLookupByName */
-NULL, /* domainSuspend */
-NULL, /* domainResume */
+openvzDomainSuspend, /* domainSuspend */
+openvzDomainResume, /* domainResume */
 openvzDomainShutdown, /* domainShutdown */
 openvzDomainReboot, /* domainReboot */
 openvzDomainShutdown, /* domainDestroy */
-- 
1.7.0.4

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

[libvirt] [PATCH] storage: kill dead stores

2010-07-30 Thread Eric Blake 
Found by clang.  Clang complained that virStorageBackendProbeTarget
could dereference NULL if backingStoreFormat was NULL, but since all
callers passed a valid pointer, I added attributes instead of null
checks.

* src/storage/storage_backend.c
(virStorageBackendQEMUImgBackingFormat): Kill dead store.
* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
Likewise.  Skip null checks, by adding attributes.
---

Thankfully, the null dereference scenario noted by clang was never
triggered in the code, which is good since it was introduced as
part of fixing a CVE.

 src/storage/storage_backend.c|3 +--
 src/storage/storage_backend_fs.c |   34 +++---
 2 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index d989743..1fe7ba6 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -581,7 +581,6 @@ static int virStorageBackendQEMUImgBackingFormat(const char 
*qemuimg)
 int newstdout = -1;
 char *help = NULL;
 enum { MAX_HELP_OUTPUT_SIZE = 1024*8 };
-int len;
 char *start;
 char *end;
 char *tmp;
@@ -591,7 +590,7 @@ static int virStorageBackendQEMUImgBackingFormat(const char 
*qemuimg)
 child, -1, newstdout, NULL, VIR_EXEC_CLEAR_CAPS)  0)
 goto cleanup;

-if ((len = virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, help))  0) {
+if (virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, help)  0) {
 virReportSystemError(errno,
  _(Unable to read '%s -h' output),
  qemuimg);
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 0761bd8..b6b8fdd 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -48,7 +48,7 @@

 #define VIR_FROM_THIS VIR_FROM_STORAGE

-static int
+static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
 virStorageBackendProbeTarget(virStorageVolTargetPtr target,
  char **backingStore,
  int *backingStoreFormat,
@@ -59,10 +59,8 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
 int fd, ret;
 virStorageFileMetadata meta;

-if (backingStore)
-*backingStore = NULL;
-if (backingStoreFormat)
-*backingStoreFormat = VIR_STORAGE_FILE_AUTO;
+*backingStore = NULL;
+*backingStoreFormat = VIR_STORAGE_FILE_AUTO;
 if (encryption)
 *encryption = NULL;

@@ -75,7 +73,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
   allocation,
   capacity))  0) {
 close(fd);
-return -1;
+return ret;
 }

 memset(meta, 0, sizeof(meta));
@@ -95,20 +93,19 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
 close(fd);

 if (meta.backingStore) {
-if (backingStore) {
-*backingStore = meta.backingStore;
-meta.backingStore = NULL;
-if (meta.backingStoreFormat == VIR_STORAGE_FILE_AUTO) {
-if ((*backingStoreFormat = 
virStorageFileProbeFormat(*backingStore))  0) {
-close(fd);
-goto cleanup;
-}
-} else {
-*backingStoreFormat = meta.backingStoreFormat;
+*backingStore = meta.backingStore;
+meta.backingStore = NULL;
+if (meta.backingStoreFormat == VIR_STORAGE_FILE_AUTO) {
+if ((*backingStoreFormat
+ = virStorageFileProbeFormat(*backingStore))  0) {
+close(fd);
+goto cleanup;
 }
 } else {
-VIR_FREE(meta.backingStore);
+*backingStoreFormat = meta.backingStoreFormat;
 }
+} else {
+VIR_FREE(meta.backingStore);
 }

 if (capacity  meta.capacity)
@@ -139,8 +136,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
 return 0;

 cleanup:
-if (backingStore)
-VIR_FREE(*backingStore);
+VIR_FREE(*backingStore);
 return -1;
 }

-- 
1.7.2

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

Re: [libvirt] [PATCH] qemu: kill some dead stores

2010-07-30 Thread Daniel Veillard 
On Fri, Jul 30, 2010 at 09:52:47AM -0600, Eric Blake wrote:
 Spotted by clang.  The qemuConnectMonitor one is an outright bug,
 the other two are cosmetic.
 
 * src/qemu/qemu_monitor.c (qemuMonitorClose): Kill dead store.
 * src/qemu/qemu_driver.c (qemudDomainSaveImageStartVM): Likewise.
 (qemuConnectMonitor): Don't lose error status.
 ---
  src/qemu/qemu_driver.c  |2 --
  src/qemu/qemu_monitor.c |4 +---
  2 files changed, 1 insertions(+), 5 deletions(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 098f4da..57b8271 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -1416,7 +1416,6 @@ qemuConnectMonitor(struct qemud_driver *driver, 
 virDomainObjPtr vm)
  ret = qemuMonitorSetCapabilities(priv-mon);
  qemuDomainObjExitMonitorWithDriver(driver, vm);
 
 -ret = 0;
  error:
  if (ret  0)
  qemuMonitorClose(priv-mon);

  Hum, if we do this we change the behaviour in case of errors in
qemuMonitorSetCapabilities(), I wonder if this wasn't there to avoid
failing in that case, and if this was the case then it's the first
affectation we need to remove. I would wait until this gets clarified
(qemuMonitorSetCapabilities failure may not be a blocking factor to
starting a guest ...)

 @@ -6535,7 +6534,6 @@ qemudDomainSaveImageStartVM(virConnectPtr conn,
  wait_ret = qemudDomainSaveImageClose(fd, read_pid, status);
  fd = -1;
  if (read_pid != -1) {
 -read_pid = -1;
  if (wait_ret == -1) {
  virReportSystemError(errno,
   _(failed to wait for process reading 
 '%s'),

  this one is fine

 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index f1494ff..2366fdb 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -685,8 +685,6 @@ cleanup:
 
  void qemuMonitorClose(qemuMonitorPtr mon)
  {
 -int refs;
 -
  if (!mon)
  return;
 
 @@ -706,7 +704,7 @@ void qemuMonitorClose(qemuMonitorPtr mon)
  mon-closed = 1;
  }
 
 -if ((refs = qemuMonitorUnref(mon))  0)
 +if (qemuMonitorUnref(mon)  0)
  qemuMonitorUnlock(mon);
  }
 

  that too obviously,

Could you push the 2 last one until we find out for first one ?

  thanks !

Daniel

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

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

Re: [libvirt] [PATCH] network: kill dead store

2010-07-30 Thread Daniel Veillard 
On Fri, Jul 30, 2010 at 10:00:46AM -0600, Eric Blake wrote:
 * src/network/bridge_driver.c (networkDefine): Kill dead store.
 ---
  src/network/bridge_driver.c |3 +--
  1 files changed, 1 insertions(+), 2 deletions(-)
 
 diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
 index b7ee880..217cdcc 100644
 --- a/src/network/bridge_driver.c
 +++ b/src/network/bridge_driver.c
 @@ -1490,14 +1490,13 @@ static virNetworkPtr networkDefine(virConnectPtr 
 conn, const char *xml) {
  virNetworkDefPtr def;
  virNetworkObjPtr network = NULL;
  virNetworkPtr ret = NULL;
 -int dupNet;
 
  networkDriverLock(driver);
 
  if (!(def = virNetworkDefParseString(xml)))
  goto cleanup;
 
 -if ((dupNet = virNetworkObjIsDuplicate(driver-networks, def, 0))  0)
 +if (virNetworkObjIsDuplicate(driver-networks, def, 0)  0)
  goto cleanup;
 
  if (virNetworkSetBridgeName(driver-networks, def, 1))

  ACK,

Daniel

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

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

[libvirt] [PATCH] Free up memballoon def.

2010-07-30 Thread Chris Lalancette 
Forgetting to do this was causing a memory leak.

Signed-off-by: Chris Lalancette clala...@redhat.com
---
 src/conf/domain_conf.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ca4bc6e..1ddea0a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -768,6 +768,8 @@ void virDomainDefFree(virDomainDefPtr def)
 
 virDomainWatchdogDefFree(def-watchdog);
 
+virDomainMemballoonDefFree(def-memballoon);
+
 virSecurityLabelDefFree(def);
 
 virCPUDefFree(def-cpu);
-- 
1.7.2

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

Re: [libvirt] [PATCH] network: kill dead store

2010-07-30 Thread Eric Blake 
On 07/30/2010 11:09 AM, Daniel Veillard wrote:
 -int dupNet;

  networkDriverLock(driver);

  if (!(def = virNetworkDefParseString(xml)))
  goto cleanup;

 -if ((dupNet = virNetworkObjIsDuplicate(driver-networks, def, 0))  0)
 +if (virNetworkObjIsDuplicate(driver-networks, def, 0)  0)
  goto cleanup;

  if (virNetworkSetBridgeName(driver-networks, def, 1))
 
   ACK,

Thanks; pushed along with the trivial hunks from the qemu patch.

-- 
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] Fix a bogus warning when parsing hostdev

2010-07-30 Thread Eric Blake 
On 07/30/2010 11:37 AM, Chris Lalancette wrote:
 When parsing hostdev, the following message would be emitted:
 
 10:17:19.052: error : virDomainHostdevDefParseXML:3748 : internal error 
 unknown node alias
 
 However, alias is appropriately parsed in
 virDomainDeviceInfoParseXML anyway.  Disable the error message
 in the initial XML parsing loop.

ACK, and thanks for the comments in the code.

-- 
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] Don't put a semicolon on the end of a VIR_ENUM_IMPL.

2010-07-30 Thread Eric Blake 
On 07/30/2010 11:37 AM, Chris Lalancette wrote:
 Signed-off-by: Chris Lalancette clala...@redhat.com
 ---
  src/conf/domain_conf.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 04c417e..ca4bc6e 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -99,7 +99,7 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, 
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
none,
pci,
drive,
 -  virtio-serial);
 +  virtio-serial)
  

ACK; practically qualifies as trivial.

-- 
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] Free up memballoon def.

2010-07-30 Thread Eric Blake 
On 07/30/2010 11:37 AM, Chris Lalancette wrote:
 Forgetting to do this was causing a memory leak.
 
 Signed-off-by: Chris Lalancette clala...@redhat.com
 ---
  src/conf/domain_conf.c |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index ca4bc6e..1ddea0a 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -768,6 +768,8 @@ void virDomainDefFree(virDomainDefPtr def)
  
  virDomainWatchdogDefFree(def-watchdog);
  
 +virDomainMemballoonDefFree(def-memballoon);

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

[libvirt] [PATCH] Update ID after stopping a domain

2010-07-30 Thread Matthias Bolte 
---
 src/esx/esx_driver.c   |1 +
 src/openvz/openvz_driver.c |1 +
 src/phyp/phyp_driver.c |2 ++
 src/vbox/vbox_tmpl.c   |1 +
 src/xenapi/xenapi_driver.c |1 +
 5 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 964a3a5..913420c 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -1620,6 +1620,7 @@ esxDomainDestroy(virDomainPtr domain)
 goto cleanup;
 }
 
+domain-id = -1;
 result = 0;
 
   cleanup:
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 98381fb..c46f3a7 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -531,6 +531,7 @@ static int openvzDomainShutdown(virDomainPtr dom) {
 
 vm-def-id = -1;
 vm-state = VIR_DOMAIN_SHUTOFF;
+dom-id = -1;
 ret = 0;
 
 cleanup:
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index e4afc5a..7143933 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -3688,6 +3688,8 @@ phypDomainDestroy(virDomainPtr dom)
 if (phypUUIDTable_RemLpar(dom-conn, dom-id) == -1)
 goto err;
 
+dom-id = -1;
+
 VIR_FREE(cmd);
 VIR_FREE(ret);
 return 0;
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 0a91c7f..31fec67 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -1587,6 +1587,7 @@ static int vboxDomainDestroy(virDomainPtr dom) {
 }
 #endif
 VBOX_RELEASE(console);
+dom-id = -1;
 ret = 0;
 }
 data-vboxSession-vtbl-Close(data-vboxSession);
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 2262cef..fb3c91d 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -830,6 +830,7 @@ xenapiDomainDestroy (virDomainPtr dom)
 return -1;
 }
 xen_vm_set_free(vms);
+dom-id = -1;
 return 0;
 }
 if (vms) xen_vm_set_free(vms);
-- 
1.7.0.4

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

Re: [libvirt] [PATCH] storage: kill dead stores

2010-07-30 Thread Matthias Bolte 
2010/7/30 Eric Blake ebl...@redhat.com:
 Found by clang.  Clang complained that virStorageBackendProbeTarget
 could dereference NULL if backingStoreFormat was NULL, but since all
 callers passed a valid pointer, I added attributes instead of null
 checks.

 * src/storage/storage_backend.c
 (virStorageBackendQEMUImgBackingFormat): Kill dead store.
 * src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
 Likewise.  Skip null checks, by adding attributes.
 ---

 Thankfully, the null dereference scenario noted by clang was never
 triggered in the code, which is good since it was introduced as
 part of fixing a CVE.

  src/storage/storage_backend.c    |    3 +--
  src/storage/storage_backend_fs.c |   34 +++---
  2 files changed, 16 insertions(+), 21 deletions(-)


 @@ -75,7 +73,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
                                                       allocation,
                                                       capacity))  0) {
         close(fd);
 -        return -1;
 +        return ret;

Why do you return ret here? Doing so doesn't harm but it's not
necessary, virStorageBackendUpdateVolTargetInfoFD returns 0 or -1
only, in contrast to virStorageBackendVolOpenCheckMode that can return
0, -1 or -2.

ACK.

Matthias

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

Re: [libvirt] [PATCH] qemu: kill some dead stores

2010-07-30 Thread Eric Blake 
On 07/30/2010 11:09 AM, Daniel Veillard wrote:
 On Fri, Jul 30, 2010 at 09:52:47AM -0600, Eric Blake wrote:
 Spotted by clang.  The qemuConnectMonitor one is an outright bug,
 the other two are cosmetic.

 * src/qemu/qemu_monitor.c (qemuMonitorClose): Kill dead store.
 * src/qemu/qemu_driver.c (qemudDomainSaveImageStartVM): Likewise.
 (qemuConnectMonitor): Don't lose error status.
 ---
  src/qemu/qemu_driver.c  |2 --
  src/qemu/qemu_monitor.c |4 +---
  2 files changed, 1 insertions(+), 5 deletions(-)

 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 098f4da..57b8271 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -1416,7 +1416,6 @@ qemuConnectMonitor(struct qemud_driver *driver, 
 virDomainObjPtr vm)
  ret = qemuMonitorSetCapabilities(priv-mon);
  qemuDomainObjExitMonitorWithDriver(driver, vm);

 -ret = 0;
  error:
  if (ret  0)
  qemuMonitorClose(priv-mon);
 
   Hum, if we do this we change the behaviour in case of errors in
 qemuMonitorSetCapabilities(), I wonder if this wasn't there to avoid
 failing in that case, and if this was the case then it's the first
 affectation we need to remove. I would wait until this gets clarified
 (qemuMonitorSetCapabilities failure may not be a blocking factor to
 starting a guest ...)

This bug was was introduced in commit e72cc3c, with dwalsh's patch on
May 27.  We were doing the correct thing before then, so only 0.8.2
contains a problem where failure to connect to the monitor is undetected.

 
   that too obviously,
 
 Could you push the 2 last one until we find out for first one ?

Yep, I've just split the patch.

-- 
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] Fix a bogus warning when parsing hostdev

2010-07-30 Thread Chris Lalancette 
When parsing hostdev, the following message would be emitted:

10:17:19.052: error : virDomainHostdevDefParseXML:3748 : internal error unknown 
node alias

However, alias is appropriately parsed in
virDomainDeviceInfoParseXML anyway.  Disable the error message
in the initial XML parsing loop.

Signed-off-by: Chris Lalancette clala...@redhat.com
---
 src/conf/domain_conf.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8721dd1..04c417e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3723,6 +3723,9 @@ virDomainHostdevDefParseXML(const xmlNodePtr node,
 goto error;
 }
 } else if (xmlStrEqual(cur-name, BAD_CAST address)) {
+/* address is parsed as part of virDomainDeviceInfoParseXML */
+} else if (xmlStrEqual(cur-name, BAD_CAST alias)) {
+/* alias is parsed as part of virDomainDeviceInfoParseXML */
 } else {
 virDomainReportError(VIR_ERR_INTERNAL_ERROR,
  _(unknown node %s), cur-name);
-- 
1.7.2

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

Re: [libvirt] [PATCH] storage: kill dead stores

2010-07-30 Thread Eric Blake 
On 07/30/2010 12:20 PM, Matthias Bolte wrote:
 @@ -75,7 +73,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
   allocation,
   capacity))  0) {
 close(fd);
 -return -1;
 +return ret;
 
 Why do you return ret here? Doing so doesn't harm but it's not
 necessary, virStorageBackendUpdateVolTargetInfoFD returns 0 or -1
 only, in contrast to virStorageBackendVolOpenCheckMode that can return
 0, -1 or -2.

The alternative was to delete the dead assignment to ret, since we would
be returning -1 ourselves.

 
 ACK.

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] Update ID after stopping a domain

2010-07-30 Thread Eric Blake 
On 07/30/2010 12:03 PM, Matthias Bolte wrote:
 ---
  src/esx/esx_driver.c   |1 +
  src/openvz/openvz_driver.c |1 +
  src/phyp/phyp_driver.c |2 ++
  src/vbox/vbox_tmpl.c   |1 +
  src/xenapi/xenapi_driver.c |1 +
  5 files changed, 6 insertions(+), 0 deletions(-)
 
 diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
 index 964a3a5..913420c 100644
 --- a/src/esx/esx_driver.c
 +++ b/src/esx/esx_driver.c
 @@ -1620,6 +1620,7 @@ esxDomainDestroy(virDomainPtr domain)
  goto cleanup;
  }
  
 +domain-id = -1;
  result = 0;

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] openvzDomainCreateWithFlags: set domain id to the correct value

2010-07-30 Thread Matthias Bolte 
2010/7/30 Matthias Bolte matthias.bo...@googlemail.com:
 2010/7/30 Jean-Baptiste Rouault jean-baptiste.roua...@diateam.net:
 I'm not sure if it's ok or not to modify dom-id as I did here.

 Regards,
 Jean-Baptiste


 I think this is a bug that needs to be fixed and the patch is correct.
 After starting a domain it has to have a valid ID = 0.

 I'm currently checking the other drivers, and the ESX and XenAPI
 drivers are affected too.

 The QEMU driver is also affected, but because it's always tunneled
 through the remote driver and the remote driver isn't affected, this
 problem doesn't surface for the QEMU driver. The same is true for the
 LXC, UML and ONE drivers.

 Matthias


ACK to your patch, I applied and pushed it.

This ID update problem exists in several other drivers too and I
posted patches to fix them.

Thanks for reporting and fixing this one, with the side effect of
revealing this as a general problem.

Matthias

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

Re: [libvirt] [PATCH] Update ID after stopping a domain

2010-07-30 Thread Matthias Bolte 
2010/7/30 Eric Blake ebl...@redhat.com:
 On 07/30/2010 12:03 PM, Matthias Bolte wrote:
 ---
  src/esx/esx_driver.c       |    1 +
  src/openvz/openvz_driver.c |    1 +
  src/phyp/phyp_driver.c     |    2 ++
  src/vbox/vbox_tmpl.c       |    1 +
  src/xenapi/xenapi_driver.c |    1 +
  5 files changed, 6 insertions(+), 0 deletions(-)

 diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
 index 964a3a5..913420c 100644
 --- a/src/esx/esx_driver.c
 +++ b/src/esx/esx_driver.c
 @@ -1620,6 +1620,7 @@ esxDomainDestroy(virDomainPtr domain)
          goto cleanup;
      }

 +    domain-id = -1;
      result = 0;

 ACK.


Thanks, pushed.

Matthias

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