Re: [libvirt] [PATCH v2 4/5] qemu: Wire up virDomainSuspendForDuration API

2012-03-16 Thread Osier Yang

On 03/16/2012 07:11 AM, Eric Blake wrote:

On 01/26/2012 12:59 PM, Michal Privoznik wrote:

This makes use of QEMU guest agent to implement the
virDomainSuspendForDuration API.
---
  src/qemu/qemu_driver.c |   93 
  1 files changed, 93 insertions(+), 0 deletions(-)



+
+if (!virDomainObjIsActive(vm)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+%s, _(domain is not running));
+goto cleanup;
+}


Same question as for quiesce: putting the guest into S3 will only work
if the agent can respond, so checking merely for active is not enough.
If the guest is active but paused, then we can't talk to the agent to
issue the request.  Having the common guest agent code check for this
condition will prevent the scenario of:

guest is paused
issue the pm suspend, but it times out


Actually I encounted the problem when playing dompmsuspend,
the dompmsuspend command hangs forever, so I tried to
suspend the guest with pm-suspend inside guest directly,
and then dompmwakeup returns quickly with success, however,
the guest wasn't waken up actually.

So there are at least two problems here:
1) dompmsuspend hangs
2) dompmwakeup returns success, while the guest wasn't waken
   up actally.

For 1), I tried to create a patch with using guest agent command
guest-ping to ping the guest agent first before executing the
guest-suspend-* commands, and hope it could return quickly if the
guest agent isn't available or something else which cause the guest
agent doesn't response, and thus we could quit quickly, but no luck,
the guest-ping hangs too.

1) could be the problem of guest-agent, while 2) might be problem
of guest kenrel. It can't be problem of guest agent, as dompmwakeup
actually uses the qemu monitor command, not guest agent command.

I didn't dig into the problems further more though, it's possible
caused by my guest agent is not setup correctly, or it's not that
newer, and has bugs. But in any case, we should handle the hanging
problem.

For anyone who wanted to reproduce the problem, the guest
is FC16.


guest is resumed
guest finally acts on command

although it is always possible that a guest will suspend itself even
without action on our part.  At least this isn't as bad as a stale
quiesce leaving the guest in a bad state, since on suspend, the host
will receive a qemu event about the change in guest state (there's no
event for freeze/thaw, since that is an aspect internal to the guest's
use of disks and not something inherently visible to qemu to generate an
event from).




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


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


[libvirt] [PATCH] fix deadlock when qemu cannot start

2012-03-16 Thread Wen Congyang
When qemu cannot start, we may call qemuProcessStop() twice.
We have check whether the vm is running at the beginning of
qemuProcessStop() to avoid libvirt deadlock. We call
qemuProcessStop() with driver and vm locked. It seems that
we can avoid libvirt deadlock. But unfortunately we may
unlock driver and vm in the function qemuProcessKill() while
vm-def-id is not -1. So qemuProcessStop() will be run twice,
and monitor will be freed unexpectedly. So we should set
vm-def-id to -1 at the beginning of qemuProcessStop().

---
 src/qemu/qemu_process.c |   20 ++--
 src/qemu/qemu_process.h |2 ++
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0af3751..44814df 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3633,10 +3633,11 @@ qemuProcessKill(struct qemud_driver *driver,
 VIR_DEBUG(vm=%s pid=%d flags=%x,
   vm-def-name, vm-pid, flags);
 
-if (!virDomainObjIsActive(vm)) {
-VIR_DEBUG(VM '%s' not active, vm-def-name);
-return 0;
-}
+if (!(flags  VIR_QEMU_PROCESS_KILL_NOCHECK))
+if (!virDomainObjIsActive(vm)) {
+VIR_DEBUG(VM '%s' not active, vm-def-name);
+return 0;
+}
 
 /* This loop sends SIGTERM (or SIGKILL if flags has
  * VIR_QEMU_PROCESS_KILL_FORCE and VIR_QEMU_PROCESS_KILL_NOWAIT),
@@ -3739,6 +3740,13 @@ void qemuProcessStop(struct qemud_driver *driver,
 return;
 }
 
+/*
+ * We may unlock driver and vm in qemuProcessKill(), so the other thread
+ * can lock driver and vm, and then call qemuProcessStop(). So we should
+ * set vm-def-id to -1 here to avoid qemuProcessStop() called twice.
+ */
+vm-def-id = -1;
+
 if ((logfile = qemuDomainCreateLog(driver, vm, true))  0) {
 /* To not break the normal domain shutdown process, skip the
  * timestamp log writing if failed on opening log file. */
@@ -3801,7 +3809,8 @@ void qemuProcessStop(struct qemud_driver *driver,
 }
 
 /* shut it off for sure */
-ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
+ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE|
+ VIR_QEMU_PROCESS_KILL_NOCHECK));
 
 /* Stop autodestroy in case guest is restarted */
 qemuProcessAutoDestroyRemove(driver, vm);
@@ -3892,7 +3901,6 @@ retry:
 
 vm-taint = 0;
 vm-pid = -1;
-vm-def-id = -1;
 virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
 VIR_FREE(priv-vcpupids);
 priv-nvcpupids = 0;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 761db6f..891f7a5 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -72,6 +72,8 @@ int qemuProcessAttach(virConnectPtr conn,
 typedef enum {
VIR_QEMU_PROCESS_KILL_FORCE  = 1  0,
VIR_QEMU_PROCESS_KILL_NOWAIT = 1  1,
+   VIR_QEMU_PROCESS_KILL_NOCHECK = 1  2, /* donot check whether the vm is
+  running */
 } virQemuProcessKillMode;
 
 int qemuProcessKill(struct qemud_driver *driver,
-- 
1.7.1

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


Re: [libvirt] Using Libvirt to change the bridge a virtual network card of a running vm is connected to

2012-03-16 Thread Hendrik Schwartke

I was sure that I did that before. Apparently I was wrong. Sorry for that.

diff --git a/.gnulib b/.gnulib
index d5612c7..e9e8aba 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit d5612c714c87555f1059d71d347e20271dced322
+Subproject commit e9e8aba12af3c903edd422fa036a356c5b2f313a
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1e56354..de54710 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -40,6 +40,8 @@
 #include qemu_cgroup.h
 #include locking/domain_lock.h
 #include network/bridge_driver.h
+#include util/virnetdev.h
+#include util/virnetdevbridge.h
 #include virnetdevtap.h

 #define VIR_FROM_THIS VIR_FROM_QEMU
@@ -1191,6 +1193,44 @@ static virDomainNetDefPtr 
qemuDomainFindNet(virDomainObjPtr vm,
 return NULL;
 }

+int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev,
+  virDomainNetDefPtr newdev)
+{
+const char *oldbridge = olddev-data.bridge.brname;
+const char *newbridge = newdev-data.bridge.brname;
+
+VIR_DEBUG(Change bridge for interface %s: %s -  %s,
+  olddev-ifname, oldbridge, newbridge);
+
+if (!virNetDevExists(newbridge)) {
+qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
+_(unable to remove port from bridge));
+return -1;
+}
+
+if (oldbridge
+virNetDevBridgeRemovePort(oldbridge, olddev-ifname)  0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(unable to remove port from bridge %s), oldbridge);
+return -1;
+}
+if (virNetDevBridgeAddPort(newbridge, newdev-ifname)  0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(unable to add port to bridge %s), newbridge);
+if (virNetDevBridgeAddPort(oldbridge, olddev-ifname)  0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(unable to recover former state by adding port to 
bridge %s), oldbridge);
+VIR_FREE(olddev-data.bridge.brname);
+}
+return -1;
+}
+VIR_FREE(olddev-data.bridge.brname);
+olddev-data.bridge.brname = newdev-data.bridge.brname;
+newdev-data.bridge.brname = NULL;
+return 0;
+}
+
+
 int qemuDomainChangeNetLinkState(struct qemud_driver *driver,
  virDomainObjPtr vm,
  virDomainNetDefPtr dev,
@@ -1321,6 +1361,13 @@ int qemuDomainChangeNet(struct qemud_driver *driver,
 return -1;
 }

+if (olddev-type == VIR_DOMAIN_NET_TYPE_BRIDGE
+  dev-type == VIR_DOMAIN_NET_TYPE_BRIDGE
+  STRNEQ_NULLABLE(olddev-data.bridge.brname, dev-data.bridge.brname)) {
+   if ((ret = qemuDomainChangeNetBridge(olddev, dev))  0)
+   return ret;
+}
+
 if (olddev-linkstate != dev-linkstate) {
 if ((ret = qemuDomainChangeNetLinkState(driver, vm, olddev, 
dev-linkstate))  0)
 return ret;
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 0310361..1e1f75c 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -80,6 +80,8 @@ int qemuDomainChangeNetLinkState(struct qemud_driver *driver,
  virDomainObjPtr vm,
  virDomainNetDefPtr dev,
  int linkstate);
+int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev,
+  virDomainNetDefPtr newdev);
 int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
   virDomainObjPtr vm,
   virDomainDeviceDefPtr dev);






On 16.03.2012 00:06, Eric Blake wrote:

On 01/26/2012 04:10 AM, Daniel P. Berrange wrote:

On Wed, Jan 25, 2012 at 04:09:22PM +0100, Hendrik Schwartke wrote:

I wrote a patch to change the mapping between a virtual bridge
interface and the host bridge while the host is up. It's based on
commit 6fba577e505611e6c25c68e322942eab7754de7e. The host and the
interface definition I used for testing are also attached.
I would be glad if the patch could be added to the repo.

Great !


...


Various whitespace fixes

  - For assignment, we prefer 'foo = bar' instead of 'foo=bar'
  - For conditionals  'if (' instead of 'if('
  - For tests  foo  0 instead of foo0


So all in all I think I'd write this as

   int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev,
 virDomainNetDefPtr newdev)
   {

...

Are you planning on resubmitting this patch with the noted improvements?



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


Re: [libvirt] [PATCH v2 4/5] qemu: Wire up virDomainSuspendForDuration API

2012-03-16 Thread Michal Privoznik
On 16.03.2012 07:52, Osier Yang wrote:
 On 03/16/2012 07:11 AM, Eric Blake wrote:
 On 01/26/2012 12:59 PM, Michal Privoznik wrote:
 This makes use of QEMU guest agent to implement the
 virDomainSuspendForDuration API.
 ---
   src/qemu/qemu_driver.c |   93
 
   1 files changed, 93 insertions(+), 0 deletions(-)

 +
 +if (!virDomainObjIsActive(vm)) {
 +qemuReportError(VIR_ERR_OPERATION_INVALID,
 +%s, _(domain is not running));
 +goto cleanup;
 +}

 Same question as for quiesce: putting the guest into S3 will only work
 if the agent can respond, so checking merely for active is not enough.
 If the guest is active but paused, then we can't talk to the agent to
 issue the request.  Having the common guest agent code check for this
 condition will prevent the scenario of:

 guest is paused
 issue the pm suspend, but it times out
 
 Actually I encounted the problem when playing dompmsuspend,
 the dompmsuspend command hangs forever, so I tried to
 suspend the guest with pm-suspend inside guest directly,
 and then dompmwakeup returns quickly with success, however,
 the guest wasn't waken up actually.
 
 So there are at least two problems here:
 1) dompmsuspend hangs
 2) dompmwakeup returns success, while the guest wasn't waken
up actally.
 
 For 1), I tried to create a patch with using guest agent command
 guest-ping to ping the guest agent first before executing the
 guest-suspend-* commands, and hope it could return quickly if the
 guest agent isn't available or something else which cause the guest
 agent doesn't response, and thus we could quit quickly, but no luck,
 the guest-ping hangs too.

This was my approach as well:

https://www.redhat.com/archives/libvir-list/2012-February/msg00055.html

Although I've used waiting with timeout; therefore making any fail
harmless. Because if we timeout on guest-sync which doesn't change the
state of guest, we don't issue any subsequent state-changing command.

I should reorder my TODO list and post the patch again. The sooner the
better.

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


[libvirt] [PATCH] libvirt: fix comment typo

2012-03-16 Thread Alex Jia
* src/libvirt.c (virStorageVolResize): correct comment typo according to
  virStorageVolResizeFlags enum define.

Signed-off-by: Alex Jia a...@redhat.com
---
 src/libvirt.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index d0b1b28..7f8d42c 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -13324,15 +13324,15 @@ error:
  * capacity; this may make the operation take noticeably longer.
  *
  * Normally, the operation treats @capacity as the new size in bytes;
- * but if @flags contains VIR_STORAGE_RESIZE_DELTA, then @capacity
+ * but if @flags contains VIR_STORAGE_VOL_RESIZE_DELTA, then @capacity
  * represents the size difference to add to the current size.  It is
  * up to the storage pool implementation whether unaligned requests are
  * rounded up to the next valid boundary, or rejected.
  *
  * Normally, this operation should only be used to enlarge capacity;
- * but if @flags contains VIR_STORAGE_RESIZE_SHRINK, it is possible to
+ * but if @flags contains VIR_STORAGE_VOL_RESIZE_SHRINK, it is possible to
  * attempt a reduction in capacity even though it might cause data loss.
- * If VIR_STORAGE_RESIZE_DELTA is also present, then @capacity is
+ * If VIR_STORAGE_VOL_RESIZE_DELTA is also present, then @capacity is
  * subtracted from the current size; without it, @capacity represents
  * the absolute new size regardless of whether it is larger or smaller
  * than the current size.
-- 
1.7.1

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


Re: [libvirt] [PATCH] libvirt: fix comment typo

2012-03-16 Thread Alex Jia

On 03/16/2012 06:44 PM, Osier Yang wrote:

On 03/16/2012 05:51 PM, Alex Jia wrote:

* src/libvirt.c (virStorageVolResize): correct comment typo according to
   virStorageVolResizeFlags enum define.


s/define/definition/

Oh, yup. Thanks and pushed with this change now.




Signed-off-by: Alex Jiaa...@redhat.com
---
  src/libvirt.c |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index d0b1b28..7f8d42c 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -13324,15 +13324,15 @@ error:
   * capacity; this may make the operation take noticeably longer.
   *
   * Normally, the operation treats @capacity as the new size in bytes;
- * but if @flags contains VIR_STORAGE_RESIZE_DELTA, then @capacity
+ * but if @flags contains VIR_STORAGE_VOL_RESIZE_DELTA, then @capacity
   * represents the size difference to add to the current size.  It is
   * up to the storage pool implementation whether unaligned requests 
are

   * rounded up to the next valid boundary, or rejected.
   *
   * Normally, this operation should only be used to enlarge capacity;
- * but if @flags contains VIR_STORAGE_RESIZE_SHRINK, it is possible to
+ * but if @flags contains VIR_STORAGE_VOL_RESIZE_SHRINK, it is 
possible to
   * attempt a reduction in capacity even though it might cause data 
loss.

- * If VIR_STORAGE_RESIZE_DELTA is also present, then @capacity is
+ * If VIR_STORAGE_VOL_RESIZE_DELTA is also present, then @capacity is
   * subtracted from the current size; without it, @capacity represents
   * the absolute new size regardless of whether it is larger or smaller
   * than the current size.


ACK with commit msg fixed.

Osier


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


Re: [libvirt] [PATCH] libvirt: fix comment typo

2012-03-16 Thread Osier Yang

On 03/16/2012 05:51 PM, Alex Jia wrote:

* src/libvirt.c (virStorageVolResize): correct comment typo according to
   virStorageVolResizeFlags enum define.


s/define/definition/



Signed-off-by: Alex Jiaa...@redhat.com
---
  src/libvirt.c |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index d0b1b28..7f8d42c 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -13324,15 +13324,15 @@ error:
   * capacity; this may make the operation take noticeably longer.
   *
   * Normally, the operation treats @capacity as the new size in bytes;
- * but if @flags contains VIR_STORAGE_RESIZE_DELTA, then @capacity
+ * but if @flags contains VIR_STORAGE_VOL_RESIZE_DELTA, then @capacity
   * represents the size difference to add to the current size.  It is
   * up to the storage pool implementation whether unaligned requests are
   * rounded up to the next valid boundary, or rejected.
   *
   * Normally, this operation should only be used to enlarge capacity;
- * but if @flags contains VIR_STORAGE_RESIZE_SHRINK, it is possible to
+ * but if @flags contains VIR_STORAGE_VOL_RESIZE_SHRINK, it is possible to
   * attempt a reduction in capacity even though it might cause data loss.
- * If VIR_STORAGE_RESIZE_DELTA is also present, then @capacity is
+ * If VIR_STORAGE_VOL_RESIZE_DELTA is also present, then @capacity is
   * subtracted from the current size; without it, @capacity represents
   * the absolute new size regardless of whether it is larger or smaller
   * than the current size.


ACK with commit msg fixed.

Osier

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


[libvirt] [PATCH libvirt-java] Return a byte[] array with secretGetValue

2012-03-16 Thread Wido den Hollander

Signed-off-by: Wido den Hollander w...@widodh.nl
---
 src/main/java/org/libvirt/Secret.java  |   11 +--
 src/main/java/org/libvirt/jna/Libvirt.java |2 +-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/libvirt/Secret.java 
b/src/main/java/org/libvirt/Secret.java
index e536cf4..a874925 100644
--- a/src/main/java/org/libvirt/Secret.java
+++ b/src/main/java/org/libvirt/Secret.java
@@ -5,6 +5,9 @@ import org.libvirt.jna.SecretPointer;
 
 import com.sun.jna.Native;
 import com.sun.jna.NativeLong;
+import com.sun.jna.ptr.LongByReference;
+import com.sun.jna.Pointer;
+import java.nio.ByteBuffer;
 
 /**
  * A secret defined by libvirt
@@ -109,9 +112,13 @@ public class Secret {
  * 
  * @return the value of the secret, or null on failure.
  */
-public String getValue() throws LibvirtException {
-String returnValue = libvirt.virSecretGetValue(VSP, new NativeLong(), 
0);
+public byte[] getValue() throws LibvirtException {
+LongByReference value_size = new LongByReference();
+Pointer value = libvirt.virSecretGetValue(VSP, value_size, 0);
 processError();
+ByteBuffer bb = value.getByteBuffer(0, value_size.getValue());
+byte[] returnValue = new byte[bb.remaining()];
+bb.get(returnValue);
 return returnValue;
 }
 
diff --git a/src/main/java/org/libvirt/jna/Libvirt.java 
b/src/main/java/org/libvirt/jna/Libvirt.java
index 2c8c03d..3804d55 100644
--- a/src/main/java/org/libvirt/jna/Libvirt.java
+++ b/src/main/java/org/libvirt/jna/Libvirt.java
@@ -330,7 +330,7 @@ public interface Libvirt extends Library {
 public int virSecretGetUUID(SecretPointer virSecretPtr, byte[] uuidString);
 public int virSecretGetUUIDString(SecretPointer virSecretPtr, byte[] 
uuidString);
 public String virSecretGetUsageID(SecretPointer virSecretPtr);   
-public String virSecretGetValue(SecretPointer virSecretPtr, NativeLong 
value_size, int flags);  
+public Pointer virSecretGetValue(SecretPointer virSecretPtr, 
LongByReference value_size, int flags);
 public String virSecretGetXMLDesc(SecretPointer virSecretPtr, int flags);  

 public SecretPointer virSecretLookupByUsage(ConnectionPointer 
virConnectPtr, int usageType, String usageID);
 public SecretPointer virSecretLookupByUUID(ConnectionPointer 
virConnectPtr, byte[] uuidBytes);
-- 
1.7.5.4

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


Re: [libvirt] Modified version of the libvirt-test-api wrapper

2012-03-16 Thread Guannan Ren

On 03/16/2012 04:24 AM, Lucas Meneghel Rodrigues wrote:

On 03/14/2012 11:56 AM, Guannan Ren wrote:

On 03/14/2012 06:45 PM, Daniel Veillard wrote:
On Tue, Mar 13, 2012 at 11:05:31PM -0300, Lucas Meneghel Rodrigues 
wrote:

Hi Guannan:

I've worked on your first version of the libvirt-test-api wrapper
for autotest. Could you please check if you like the modified
version?

https://github.com/autotest/autotest/pull/230

If you do think it's fine, you can ack it, or you might take it,
modify and resend it. On a git branch, you can do the following:

curl https://github.com/autotest/autotest/pull/230.patch | git am

And then modify and resend.

All this looks fine to me. Maybe the libvirt-test-API.tar.gz
should be renamed to stamp it, we should make sure it is recent, and
possibly make a release (that could be as simple as copying one
of
ftp://libvirt.org/libvirt/libvirt-test-API/libvirt-test-API-git-snapshot.tar.gz 



and renaming it as libvirt-test-API-0.1.0.tar.gz) and push that to
autotest git instead,

Daniel



I agree with this, for following work,
we could submit patch to autotest mailing list.

Guannan


Ok, so I've commited the wrapper to the upstream repo:

https://github.com/autotest/autotest/commit/c915fa8aea2c6a0542f2d798ad6a0fbaf2738ef3 



When you want to follow up with this, let me know.

Cheers,

Lucas


That's great.
Thanks.

Guannan Ren

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


Re: [libvirt] [PATCH] remote: Fix migration leaks

2012-03-16 Thread Daniel Veillard
On Fri, Mar 16, 2012 at 07:46:16PM +0800, Osier Yang wrote:
 How to reproduce:
 
 % valgrind -v --leak-check=full virsh migrate mig \
   qemu+ssh://$dest/system --unsafe
 
 == 8 bytes in 1 blocks are definitely lost in loss record 1 of 28
 ==at 0x4A04A28: calloc (vg_replace_malloc.c:467)
 ==by 0x3EB7115FB8: xdr_reference (in /lib64/libc-2.12.so)
 ==by 0x3EB7115F10: xdr_pointer (in /lib64/libc-2.12.so)
 ==by 0x4D1EA84: xdr_remote_string (remote_protocol.c:40)
 ==by 0x4D1EAD8: xdr_remote_domain_migrate_prepare3_ret 
 (remote_protocol.c:4772)
 ==by 0x4D2FFD2: virNetMessageDecodePayload (virnetmessage.c:382)
 ==by 0x4D2789C: virNetClientProgramCall (virnetclientprogram.c:382)
 ==by 0x4D0707D: callWithFD (remote_driver.c:4549)
 ==by 0x4D070FB: call (remote_driver.c:4570)
 ==by 0x4D12AEE: remoteDomainMigratePrepare3 (remote_driver.c:4138)
 ==by 0x4CF7BE9: virDomainMigrateVersion3 (libvirt.c:4815)
 ==by 0x4CF9432: virDomainMigrate2 (libvirt.c:5454)
 ==
 == LEAK SUMMARY:
 ==definitely lost: 8 bytes in 1 blocks
 ==indirectly lost: 0 bytes in 0 blocks
 ==  possibly lost: 0 bytes in 0 blocks
 ==still reachable: 126,995 bytes in 1,343 blocks
 == suppressed: 0 bytes in 0 blocks
 
 This patch also fixes the leaks in remoteDomainMigratePrepare and
 remoteDomainMigratePrepare2.
 ---
  src/remote/remote_driver.c |3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)
 
 diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
 index 031167a..4ddebcb 100644
 --- a/src/remote/remote_driver.c
 +++ b/src/remote/remote_driver.c
 @@ -1963,6 +1963,7 @@ remoteDomainMigratePrepare (virConnectPtr dconn,
  if (ret.uri_out)
  *uri_out = *ret.uri_out; /* Caller frees. */
  
 +VIR_FREE(ret.uri_out);
  rv = 0;
  
  done:
 @@ -2018,6 +2019,7 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn,
  rv = 0;
  
  done:
 +VIR_FREE(ret.uri_out);
  remoteDriverUnlock(priv);
  return rv;
  error:
 @@ -4161,6 +4163,7 @@ remoteDomainMigratePrepare3(virConnectPtr dconn,
  rv = 0;
  
  done:
 +VIR_FREE(ret.uri_out);
  remoteDriverUnlock(priv);
  return rv;
  error:

  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] remote: Fix migration leaks

2012-03-16 Thread Osier Yang
How to reproduce:

% valgrind -v --leak-check=full virsh migrate mig \
  qemu+ssh://$dest/system --unsafe

== 8 bytes in 1 blocks are definitely lost in loss record 1 of 28
==at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==by 0x3EB7115FB8: xdr_reference (in /lib64/libc-2.12.so)
==by 0x3EB7115F10: xdr_pointer (in /lib64/libc-2.12.so)
==by 0x4D1EA84: xdr_remote_string (remote_protocol.c:40)
==by 0x4D1EAD8: xdr_remote_domain_migrate_prepare3_ret 
(remote_protocol.c:4772)
==by 0x4D2FFD2: virNetMessageDecodePayload (virnetmessage.c:382)
==by 0x4D2789C: virNetClientProgramCall (virnetclientprogram.c:382)
==by 0x4D0707D: callWithFD (remote_driver.c:4549)
==by 0x4D070FB: call (remote_driver.c:4570)
==by 0x4D12AEE: remoteDomainMigratePrepare3 (remote_driver.c:4138)
==by 0x4CF7BE9: virDomainMigrateVersion3 (libvirt.c:4815)
==by 0x4CF9432: virDomainMigrate2 (libvirt.c:5454)
==
== LEAK SUMMARY:
==definitely lost: 8 bytes in 1 blocks
==indirectly lost: 0 bytes in 0 blocks
==  possibly lost: 0 bytes in 0 blocks
==still reachable: 126,995 bytes in 1,343 blocks
== suppressed: 0 bytes in 0 blocks

This patch also fixes the leaks in remoteDomainMigratePrepare and
remoteDomainMigratePrepare2.
---
 src/remote/remote_driver.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 031167a..4ddebcb 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1963,6 +1963,7 @@ remoteDomainMigratePrepare (virConnectPtr dconn,
 if (ret.uri_out)
 *uri_out = *ret.uri_out; /* Caller frees. */
 
+VIR_FREE(ret.uri_out);
 rv = 0;
 
 done:
@@ -2018,6 +2019,7 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn,
 rv = 0;
 
 done:
+VIR_FREE(ret.uri_out);
 remoteDriverUnlock(priv);
 return rv;
 error:
@@ -4161,6 +4163,7 @@ remoteDomainMigratePrepare3(virConnectPtr dconn,
 rv = 0;
 
 done:
+VIR_FREE(ret.uri_out);
 remoteDriverUnlock(priv);
 return rv;
 error:
-- 
1.7.1

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


Re: [libvirt] [PATCH] remote: Fix migration leaks

2012-03-16 Thread Osier Yang

On 03/16/2012 07:02 PM, Daniel Veillard wrote:

On Fri, Mar 16, 2012 at 07:46:16PM +0800, Osier Yang wrote:

How to reproduce:

% valgrind -v --leak-check=full virsh migrate mig \
   qemu+ssh://$dest/system --unsafe

== 8 bytes in 1 blocks are definitely lost in loss record 1 of 28
==at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==by 0x3EB7115FB8: xdr_reference (in /lib64/libc-2.12.so)
==by 0x3EB7115F10: xdr_pointer (in /lib64/libc-2.12.so)
==by 0x4D1EA84: xdr_remote_string (remote_protocol.c:40)
==by 0x4D1EAD8: xdr_remote_domain_migrate_prepare3_ret 
(remote_protocol.c:4772)
==by 0x4D2FFD2: virNetMessageDecodePayload (virnetmessage.c:382)
==by 0x4D2789C: virNetClientProgramCall (virnetclientprogram.c:382)
==by 0x4D0707D: callWithFD (remote_driver.c:4549)
==by 0x4D070FB: call (remote_driver.c:4570)
==by 0x4D12AEE: remoteDomainMigratePrepare3 (remote_driver.c:4138)
==by 0x4CF7BE9: virDomainMigrateVersion3 (libvirt.c:4815)
==by 0x4CF9432: virDomainMigrate2 (libvirt.c:5454)
==
== LEAK SUMMARY:
==definitely lost: 8 bytes in 1 blocks
==indirectly lost: 0 bytes in 0 blocks
==  possibly lost: 0 bytes in 0 blocks
==still reachable: 126,995 bytes in 1,343 blocks
== suppressed: 0 bytes in 0 blocks

This patch also fixes the leaks in remoteDomainMigratePrepare and
remoteDomainMigratePrepare2.
---
  src/remote/remote_driver.c |3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 031167a..4ddebcb 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1963,6 +1963,7 @@ remoteDomainMigratePrepare (virConnectPtr dconn,
  if (ret.uri_out)
  *uri_out = *ret.uri_out; /* Caller frees. */

+VIR_FREE(ret.uri_out);
  rv = 0;

  done:
@@ -2018,6 +2019,7 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn,
  rv = 0;

  done:
+VIR_FREE(ret.uri_out);
  remoteDriverUnlock(priv);
  return rv;
  error:
@@ -4161,6 +4163,7 @@ remoteDomainMigratePrepare3(virConnectPtr dconn,
  rv = 0;

  done:
+VIR_FREE(ret.uri_out);
  remoteDriverUnlock(priv);
  return rv;
  error:


   ACK :-)

Daniel



Pushed, Thanks!

Osier

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


[libvirt] [PATCH] Fix handling of blkio deviceWeight empty string

2012-03-16 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

A common coding pattern for changing blkio parameters is

  1. virDomainGetBlkioParameters

  2. change one or more params

  3. virDomainSetBlkioParameters

For this to work, it must be possible to roundtrip through
the methods without error. Unforuntelty virDomainGetBlkioParameters
will return  for the deviceWeight parameter for guests by default,
which virDomainSetBlkioParameters will then reject as invalid.

This fixes the handling of  to be a no-op, and also improves the
error message to tell you what was invalid
---
 src/qemu/qemu_driver.c |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b20b3e9..2c467ab 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5959,6 +5959,12 @@ qemuDomainParseDeviceWeightStr(char *deviceWeightStr,
 int i;
 virBlkioDeviceWeightPtr result = NULL;
 
+*dw = NULL;
+*size = 0;
+
+if (STREQ(deviceWeightStr, ))
+return 0;
+
 temp = deviceWeightStr;
 while (temp) {
 temp = strchr(temp, ',');
@@ -6021,7 +6027,7 @@ qemuDomainParseDeviceWeightStr(char *deviceWeightStr,
 
 error:
 qemuReportError(VIR_ERR_INVALID_ARG,
-_(unable to parse %s), deviceWeightStr);
+_(unable to parse device weight '%s'), deviceWeightStr);
 cleanup:
 virBlkioDeviceWeightArrayClear(result, ndevices);
 VIR_FREE(result);
-- 
1.7.7.6

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


Re: [libvirt] [PATCH] Increased upper limit on lists of pool names

2012-03-16 Thread Eric Blake
On 03/15/2012 08:26 PM, Jesse Cook wrote:
 
 v0.9.10 client did not want to play nicely with the v0.9.10 server via
 qemu+ssh.  I got frustrated and just tried running the test from a
 client running an older version of libvirt.  When connecting to
 v0.9.10, it behaved the same way the pre-patched did in my cover
 letter.  I don't have full test results because of the communication
 errors I was getting. I'll try to either figure it out tomorrow or
 just use the older version as the client (pre-patch and patch).

Since qemu already uses rpc mechanism, I've found it easy to
mix-and-match by doing this in two terminals both logged in as root, and
both located in a libvirt.git checkout directory (if you don't mind
building as root; if you do mind building as root, then have three,
where the third is a regular user running as non-root to do the builds):

setup:
1# git clone libvirt ...
1# cd libvirt
1# ./autobuild.sh --system
1# apply patches I'm interested in testing
1# make
2# cd libvirt

old libvirtd server, old virsh client:
1# service libvirtd start
2# virsh ...
1# service libvirtd stop

old libvirtd, new virsh
1# service libvirtd start
2# tools/virsh ...
1# service libvirtd stop

new libvirtd, old virsh
1# daemon/libvirtd
2# virsh ...
1# Ctrl-C

new libvirtd, new virsh
1# daemon/libvirtd
2# tools/virsh ...
1# Ctrl-C

That gives me testing of all four protocol combinations, assuming that
the installed version is a current release without my patch, and my
just-built in-tree version has my patch.  No need to complicate protocol
testing with multi-host qemu+ssh:// stuff.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] libvirt: xen: do not use ioemu type for any emulated NIC

2012-03-16 Thread Stefan Bader
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

When using the xm/xend stack to manage instances there is a bug that causes
the emulated interfaces to be unusable when the vif config contains type=ioemu.

The current code already has a special quirk to not use this keyword if no
specific model is given for the emulated NIC (defaulting to rtl8139).
Essentially it works because regardless of the type argument, the Xen stack
always creates emulated and paravirt interfaces and lets the guest decide
which one to use. So neither xl nor xm stack actually require the type keyword
for emulated NICs.

- ---
/*
 * apparently type ioemu breaks paravirt drivers on HVM so skip this
 * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU
 */
 if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
 virBufferAddLit(buf, ,type=ioemu);
- ---

This is still broken when a different emulated NIC (eg. e1000 is enforced),
but the same fix solves that problem. Which is what the attached patch does.

I have been testing with this change applied and was able to change between
various emulated NIC models successfully. I believe it would be applicable
to libvirt upstream, too.

- -Stefan

PS: Please cc: me directly on replies as I am not subscribed to this list. 
Thanks.


diff -Nurp libvirt.old/src/xenxs/xen_sxpr.c libvirt-0.9.8/src/xenxs/xen_sxpr.c
- --- libvirt.old/src/xenxs/xen_sxpr.c2012-03-16 11:17:02.685042296 +0100
+++ libvirt-0.9.8/src/xenxs/xen_sxpr.c  2012-03-15 18:43:32.0 +0100
@@ -1873,7 +1873,9 @@ xenFormatSxprNet(virConnectPtr conn,
 }
 else {
 virBufferEscapeSexpr(buf, (model '%s'), def-model);
- -virBufferAddLit(buf, (type ioemu));
+   /* See above. Also needed when model is specified. */
+if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
+virBufferAddLit(buf, (type ioemu));
 }

 if (!isAttach)
diff -Nurp libvirt.old/src/xenxs/xen_xm.c libvirt-0.9.8/src/xenxs/xen_xm.c
- --- libvirt.old/src/xenxs/xen_xm.c  2012-03-16 11:17:02.645042103 +0100
+++ libvirt-0.9.8/src/xenxs/xen_xm.c2012-03-15 18:43:32.0 +0100
@@ -1335,7 +1335,9 @@ static int xenFormatXMNet(virConnectPtr
 }
 else {
 virBufferAsprintf(buf, ,model=%s, net-model);
- -virBufferAddLit(buf, ,type=ioemu);
+   /* See above. Also needed if model is specified. */
+if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
+virBufferAddLit(buf, ,type=ioemu);
 }

 if (net-ifname)
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBCgAGBQJPYxc9AAoJEOhnXe7L7s6j3f8P/3nyZUh5dY8vVcGsRX9mkoX5
DwioJfLit8wjNUX3iJciPnTCUprJeiOpev/dKuuIaUfSEOP+xzHKcAbSVdyXhlME
R5n+EbBI6WBDbVT/rbV+Mp52bVoe3vb8LgT2dc1KzYYpcMjlzAzUd56IdJ64YQaS
r9o6ps279jY6AeDEEnz0TTajKdNbMBt578kwmsES/1rHMVLT2wLrnQp6Kc9ECQ5o
YjpOAU55Bl8OP+1DK0SFSyRl1gesrUXCN1g6QsWiiaeq0OpO8TDxfpxoBKwV19cO
kFHoZ8UR4XyaDosVg0S2+WaOn7o8B7r1k+biD23L4QaADcsN+l4uuDHaHYX4nd/L
PpbUpZY8+KjTny6H/khqYYJR/G9Wl7L6GMlGLJLihlue3/iIfrHJZdF1+dFgo9uZ
vaYgjZWe1ak9vQIUfxa4enxdadBYS7Xy1JP9egA87DACnpzOEWgZ6Qadhcj0cewm
MHfEmFWnaHHLI+TqaLSwN3XeTsbZ2GP8ixlW+ewNyAPd76g3uYFVTxDOd9a+r301
C+jtgwDcG1aciXR/diR/Dnr7JZ3Y9Fgu/gt/n0Pr4tIgiYF8+T4WG0osXssiiqbc
3p92VzkEl3P/g5QRdeB/+XxGz9IfAikALyNO4ztFSxk4JIh1ix/OIoaucNSc7XFl
boPeTDnZLrdIVncDmJ5c
=RhMW
-END PGP SIGNATURE-
diff -Nurp libvirt.old/src/xenxs/xen_sxpr.c libvirt-0.9.8/src/xenxs/xen_sxpr.c
--- libvirt.old/src/xenxs/xen_sxpr.c	2012-03-16 11:17:02.685042296 +0100
+++ libvirt-0.9.8/src/xenxs/xen_sxpr.c	2012-03-15 18:43:32.0 +0100
@@ -1873,7 +1873,9 @@ xenFormatSxprNet(virConnectPtr conn,
 }
 else {
 virBufferEscapeSexpr(buf, (model '%s'), def-model);
-virBufferAddLit(buf, (type ioemu));
+	/* See above. Also needed when model is specified. */
+if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
+virBufferAddLit(buf, (type ioemu));
 }
 
 if (!isAttach)
diff -Nurp libvirt.old/src/xenxs/xen_xm.c libvirt-0.9.8/src/xenxs/xen_xm.c
--- libvirt.old/src/xenxs/xen_xm.c	2012-03-16 11:17:02.645042103 +0100
+++ libvirt-0.9.8/src/xenxs/xen_xm.c	2012-03-15 18:43:32.0 +0100
@@ -1335,7 +1335,9 @@ static int xenFormatXMNet(virConnectPtr
 }
 else {
 virBufferAsprintf(buf, ,model=%s, net-model);
-virBufferAddLit(buf, ,type=ioemu);
+	/* See above. Also needed if model is specified. */
+if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
+virBufferAddLit(buf, ,type=ioemu);
 }
 
 if (net-ifname)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Fix handling of blkio deviceWeight empty string

2012-03-16 Thread Eric Blake
On 03/16/2012 05:07 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 A common coding pattern for changing blkio parameters is
 
   1. virDomainGetBlkioParameters
 
   2. change one or more params
 
   3. virDomainSetBlkioParameters
 
 For this to work, it must be possible to roundtrip through
 the methods without error. Unforuntelty virDomainGetBlkioParameters

s/Unforuntelty/Unfortunately/

 will return  for the deviceWeight parameter for guests by default,
 which virDomainSetBlkioParameters will then reject as invalid.
 
 This fixes the handling of  to be a no-op, and also improves the
 error message to tell you what was invalid
 ---
  src/qemu/qemu_driver.c |8 +++-
  1 files changed, 7 insertions(+), 1 deletions(-)

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] Add migration APIs for libxl driver

2012-03-16 Thread Jim Fehlig
Chunyan Liu wrote:

 2012/3/15 Jim Fehlig jfeh...@suse.com mailto:jfeh...@suse.com


  +/* Create socket connection to receive migration data */
  +if (!uri_in) {
  +hostname = virGetHostname(dconn);
  +if (hostname == NULL)
  +goto cleanup;
  +
  +port = libxlFindFreeMigPort(driver,
 LIBXL_MIGRATION_MIN_PORT);
 

 I think you should reserve the port in libxlFindFreeMigPort(), similar
 to libxlNextFreeVncPort().  In fact, you could probably generalize
 libxlNextFreeVncPort(), e.g. libxlNextFreePort(virBitmapPtr
 bitmap, int
 start_port, int stop_port) and use it to find available VNC and
 migration ports.


 There is some difference to handle Migration ports and VNC ports:
 VNC port always find a free port from VNC ports range and use it, but
 migration port could be pointed by user or if not pointed find a free
 port to use it. There are two places need to set bitmap maybe:
 1. The port pointed by user could be a port in default migration ports
 range, we should set bitmap so that next time finding free port could
 avoid that port.

If the user is specifying the port, we should just use it and be done. 
That is how the VNC port is handled too.  If user has specified a vnc
port, then we just use it.  Otherwise, call libxlNextFreeVncPort to find
a free one.

 2. No port pointed by user, then find a free migration port from
 default migration ports range, and set bitmap.
 Besides, with port pointed, we need to create socket and bind to the
 port too. libxlFindFreeVNCPort creates socket and binds port and set
 bitmap in the function, if FindFreeMigPort also does that, then to
 user pointed port, we need to do same work again.

libxlFindFreeVNCPort only binds to the port to see if it is in use.  If
not in use, it closes the socket, sets the corresponding bit in the
bitmap, and returns the port.  Caller then knows the port is free and
available for use, e.g. binding, listening, connecting, or whatever it
pleases to do with the port.

IMO, we could have

static int libxlNextFreePort(virtBitmapPtr bitmap, int startPort, int
numPorts)

which is functionally equivalent to libxlNextFreeVncPort(), but examines
startPort through startPort+numPorts.

Thanks,
Jim

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


[libvirt] [PATCH v2] qemu_agent: Issue guest-sync prior to every command

2012-03-16 Thread Michal Privoznik
If we issue guest command and GA is not running, the issuing thread
will block endlessly. We can check for GA presence by issuing
guest-sync with unique ID (timestamp). We don't want to issue real
command as even if GA is not running, once it is started, it process
all commands written to GA socket.
---
diff to v1:
- don't keep list of issued IDs because it's pointless

Some background on this:
I've intended to switch to new guest-sync-delimited and use
older guest-sync for older GA. However, since we don't use
stream base implementation but use new line as delimiter for
GA responses we don't need GA to issue sentinel byte 0xFF for us:

  http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol

Moreover, since we are using guest-sync just for detecting GA, it is
not necessary to use sentinel byte at all:

  http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03278.html

 src/qemu/qemu_agent.c |  134 ++--
 1 files changed, 128 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index a17d025..ee82fae 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -39,6 +39,7 @@
 #include virterror_internal.h
 #include json.h
 #include virfile.h
+#include virtime.h
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -316,6 +317,7 @@ qemuAgentIOProcessLine(qemuAgentPtr mon,
 {
 virJSONValuePtr obj = NULL;
 int ret = -1;
+unsigned long long id;
 
 VIR_DEBUG(Line [%s], line);
 
@@ -340,6 +342,20 @@ qemuAgentIOProcessLine(qemuAgentPtr mon,
 obj = NULL;
 ret = 0;
 } else {
+/* If we've received something like:
+ *  {return: 1234}
+ * it is likely that somebody started GA
+ * which is now processing our previous
+ * guest-sync commands. Check if this is
+ * the case and don't report an error but
+ * return silently.
+ */
+if (virJSONValueObjectGetNumberUlong(obj, return, id) == 0) {
+VIR_DEBUG(Ignoring delayed reply to guest-sync: %llu, id);
+ret = 0;
+goto cleanup;
+}
+
 qemuReportError(VIR_ERR_INTERNAL_ERROR,
 _(Unexpected JSON reply '%s'), line);
 }
@@ -813,11 +829,28 @@ void qemuAgentClose(qemuAgentPtr mon)
 qemuAgentUnlock(mon);
 }
 
+#define QEMU_AGENT_WAIT_TIME (1000ull * 5)
 
+/**
+ * qemuAgentSend:
+ * @mon: Monitor
+ * @msg: Message
+ * @timeout: use timeout?
+ *
+ * Send @msg to agent @mon.
+ * Wait max QEMU_AGENT_WAIT_TIME for agent
+ * to reply.
+ *
+ * Returns: 0 on success,
+ *  -2 on timeout,
+ *  -1 otherwise
+ */
 static int qemuAgentSend(qemuAgentPtr mon,
- qemuAgentMessagePtr msg)
+ qemuAgentMessagePtr msg,
+ bool timeout)
 {
 int ret = -1;
+unsigned long long now, then;
 
 /* Check whether qemu quit unexpectedly */
 if (mon-lastError.code != VIR_ERR_OK) {
@@ -827,13 +860,26 @@ static int qemuAgentSend(qemuAgentPtr mon,
 return -1;
 }
 
+if (timeout) {
+if (virTimeMillisNow(now)  0)
+return -1;
+then = now + QEMU_AGENT_WAIT_TIME;
+}
+
 mon-msg = msg;
 qemuAgentUpdateWatch(mon);
 
 while (!mon-msg-finished) {
-if (virCondWait(mon-notify, mon-lock)  0) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
-_(Unable to wait on monitor condition));
+if ((timeout  virCondWaitUntil(mon-notify, mon-lock, then)  0) 
||
+(!timeout  virCondWait(mon-notify, mon-lock)  0)) {
+if (errno == ETIMEDOUT) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(Guest agent not available for now));
+ret = -2;
+} else {
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(Unable to wait on monitor condition));
+}
 goto cleanup;
 }
 }
@@ -855,6 +901,78 @@ cleanup:
 }
 
 
+/**
+ * qemuAgentGuestSync:
+ * @mon: Monitor
+ *
+ * Send guest-sync with unique ID
+ * and wait for reply. If we get one, check if
+ * received ID is equal to given.
+ *
+ * Returns: 0 on success,
+ *  -1 otherwise
+ */
+static int
+qemuAgentGuestSync(qemuAgentPtr mon)
+{
+int ret = -1;
+int send_ret;
+unsigned long long id, id_ret;
+qemuAgentMessage sync_msg;
+
+memset(sync_msg, 0, sizeof sync_msg);
+
+if (virTimeMillisNow(id)  0)
+return -1;
+
+if (virAsprintf(sync_msg.txBuffer,
+{\execute\:\guest-sync\, 
+\arguments\:{\id\:%llu}}, id)  0) {
+virReportOOMError();
+return -1;
+}
+
+sync_msg.txLength = strlen(sync_msg.txBuffer);
+
+VIR_DEBUG(Sending guest-sync command with ID: %llu, id);
+
+   

[libvirt] heisenbug in command.c

2012-03-16 Thread Serge Hallyn
Hi,

It seems I've run into quite the heisenbug, reported at
https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/922628

It manifests itself as virPidWait returning status=4 for iptables (which
should never exit with status=4).  But it's only been seen on two (very
different) machines, and the slightest shifting of the winds makes it go
away.  Given how sneaky this bug appears to be, there's a slight
temptation to have iptablesAddRemoveRule pass in a int* for status and
better deal with the -EINTR.  But I fear that might be papering over a
worse race.

Does anyone have any ideas on this?  Has anyone else ever seen this?

-serge

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


[libvirt] [libvirt-glib] Remove unneeded call in libvirt-gconfig test

2012-03-16 Thread Christophe Fergeau
Because of a copy and paste error, the test program is making an
unneeded call to gvir_config_domain_channel_set_target_type
when setting up its USB redirection device.
---
 libvirt-gconfig/tests/test-domain-create.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/libvirt-gconfig/tests/test-domain-create.c 
b/libvirt-gconfig/tests/test-domain-create.c
index 88c887d..4ee33aa 100644
--- a/libvirt-gconfig/tests/test-domain-create.c
+++ b/libvirt-gconfig/tests/test-domain-create.c
@@ -204,8 +204,6 @@ int main(int argc, char **argv)
 redirdev = gvir_config_domain_redirdev_new();
 gvir_config_domain_redirdev_set_bus(redirdev,
 GVIR_CONFIG_DOMAIN_REDIRDEV_BUS_USB);
-gvir_config_domain_channel_set_target_type(channel,
-   
GVIR_CONFIG_DOMAIN_CHANNEL_TARGET_VIRTIO);
 spicevmc = gvir_config_domain_chardev_source_spicevmc_new();
 gvir_config_domain_chardev_set_source(GVIR_CONFIG_DOMAIN_CHARDEV(redirdev),
   
GVIR_CONFIG_DOMAIN_CHARDEV_SOURCE(spicevmc));
-- 
1.7.7.6

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


Re: [libvirt] heisenbug in command.c

2012-03-16 Thread Eric Blake
On 03/16/2012 10:36 AM, Serge Hallyn wrote:
 Hi,
 
 It seems I've run into quite the heisenbug, reported at
 https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/922628
 
 It manifests itself as virPidWait returning status=4 for iptables (which
 should never exit with status=4).

Maybe iptables isn't documented as exiting with $? of 4, but that's what
is happening.  The libvirt code in question is quite clear that it
grabbed an accurate exit status from the child process.

ret = virPidWait(cmd-pid, exitstatus ? exitstatus : status);
if (ret == 0) {
cmd-pid = -1;
cmd-reap = false;
if (status) {
char *str = virCommandToString(cmd);
char *st = virCommandTranslateStatus(status);
virCommandError(VIR_ERR_INTERNAL_ERROR,
_(Child process (%s) status unexpected: %s),
str ? str : cmd-args[0], NULLSTR(st));

  But it's only been seen on two (very
 different) machines, and the slightest shifting of the winds makes it go
 away.  Given how sneaky this bug appears to be, there's a slight
 temptation to have iptablesAddRemoveRule pass in a int* for status and
 better deal with the -EINTR.  But I fear that might be papering over a
 worse race.

I don't follow how you think there is a -EINTR being encountered in
libvirt.  I think you'd be better off investigating why iptables really
is exiting with status 4.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] heisenbug in command.c

2012-03-16 Thread Serge Hallyn

On 03/16/2012 11:50 AM, Eric Blake wrote:

On 03/16/2012 10:36 AM, Serge Hallyn wrote:

Hi,

It seems I've run into quite the heisenbug, reported at
https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/922628

It manifests itself as virPidWait returning status=4 for iptables (which
should never exit with status=4).


Maybe iptables isn't documented as exiting with $? of 4, but that's what
is happening.  The libvirt code in question is quite clear that it
grabbed an accurate exit status from the child process.



Well, yes.  I figured that either (1) iptables actually got -EINTR from 
the kernel and passed that along as its exit code, or (2) something went 
wrong with memory being overwritten in libvirt, however unlikely. 
Stranger things have happened.  If (1), I was wondering if it was being 
ignored on purpose.



 ret = virPidWait(cmd-pid, exitstatus ? exitstatus :status);
 if (ret == 0) {
 cmd-pid = -1;
 cmd-reap = false;
 if (status) {
 char *str = virCommandToString(cmd);
 char *st = virCommandTranslateStatus(status);
 virCommandError(VIR_ERR_INTERNAL_ERROR,
 _(Child process (%s) status unexpected: %s),
 str ? str : cmd-args[0], NULLSTR(st));


  But it's only been seen on two (very
different) machines, and the slightest shifting of the winds makes it go
away.  Given how sneaky this bug appears to be, there's a slight
temptation to have iptablesAddRemoveRule pass in a int* for status and
better deal with the -EINTR.  But I fear that might be papering over a
worse race.


I don't follow how you think there is a -EINTR being encountered in
libvirt.


Yeah I don't really either.

 I think you'd be better off investigating why iptables really

is exiting with status 4.


Well, given what EINTR means, shouldn't src/util/iptables.c re-try the 
command if it gets that?


Anyway I'll keep digging, but was wondering if anyone else has seen this.

-serge

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


Re: [libvirt] [PATCH v2] python: add virDomainGetCPUStats python binding API

2012-03-16 Thread Eric Blake
On 03/14/2012 07:03 AM, Guannan Ren wrote:
 dom.getCPUStats(True, 0)
   [{'cpu_time': 92913537401L, 'system_time': 547000L, 'user_time': 
 31000L}]
 
 dom.getCPUStats(False, 0)
   [{'cpu_time': 39476858499L}, {'cpu_time': 10627048370L}, {'cpu_time': 
 21270945682L}, {'cpu_time': 21556420641L}]
 
 *generator.py Add a new naming rule
 *libvirt-override-api.xml The API function description
 *libvirt-override.c Implement it.
 ---
  python/generator.py |5 +-
  python/libvirt-override-api.xml |   10 +++
  python/libvirt-override.c   |  164 
 +++
  3 files changed, 178 insertions(+), 1 deletions(-)
 
 +++ b/python/libvirt-override-api.xml
 @@ -149,6 +149,16 @@
arg name='path' type='char *' info='the path for the block device'/
arg name='flags' type='int' info='flags (unused; pass 0)'/
  /function
 +function name='virDomainGetCPUStats' file='python'
 +  infoExtracts CPU statistics for a running domain, On success it will 
 return a list of data of dictionary type.

s/, On/. On/

Long lines; can you wrap this to fit in 80 columns?

 +   If boolean total is True, the first element of the list refers to CPU0 on 
 the host, second element is CPU1, and so on.

s/total is True/total is False/

 +   The format of data struct is like [{cpu_time:xxx},{cpu_time:xxx}, ...]
 +   If it is False, it returns total domain CPU statistics like 
 [{cpu_time:xxx, user_time:xxx, system_time:xxx}]/info

s/False/True/

 +
 +if (!PyBool_Check(totalbool)) {
 +PyErr_Format(PyExc_TypeError,
 +The \total\ attribute must be bool);
 +return NULL;
 +}
 +
 +if ((ret = PyList_New(0)) == NULL)
 +return NULL;
 +
 +if (totalbool == Py_False) {

Per other code in libvirt-override.c, you can't compare totalbool (type
PyObject) with Py_False, at least not on all compilers.  You need
something like this instead:

/* Hack - Python's definition of Py_True breaks strict
 * aliasing rules, so can't directly compare
 */
if (PyBool_Check(value)) {
PyObject *hacktrue = PyBool_FromLong(1);
temp-value.b = hacktrue == value ? 1 : 0;
Py_DECREF(hacktrue);

 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +ncpus = virDomainGetCPUStats(domain, NULL, 0, 0, 0, flags);
 +LIBVIRT_END_ALLOW_THREADS;
 +
 +if (ncpus  0) {
 +error = VIR_PY_NONE;
 +goto failed;
 +}
 +
 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +nparams = virDomainGetCPUStats(domain, NULL, 0, 0, 1, flags);
 +LIBVIRT_END_ALLOW_THREADS;
 +
 +if (nparams  0) {
 +error = VIR_PY_NONE;
 +goto failed;
 +}

So far, so good.

 +
 +if (!nparams) {
 +if ((cpu = PyDict_New()) == NULL) {
 +error = NULL;

Initialize error to NULL at the front, and you won't have to set it here.

 +goto failed;
 +}
 +
 +if (PyList_Append(ret, cpu)  0) {
 +Py_DECREF(cpu);
 +error = NULL;
 +goto failed;
 +}
 +
 +Py_DECREF(cpu);
 +return ret;

So why are we populating the list with a single empty dictionary?
Shouldn't we instead be populating it with one empty dictionary per cpu?
 That is, this code returns [{}], but if ncpus is 4, it should return
[{},{},{},{}].

 +}
 +sumparams = nparams * ncpus;
 +
 +if (VIR_ALLOC_N(params, sumparams)  0) {
 +error = PyErr_NoMemory();
 +goto failed;
 +}
 +
 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +i_retval = virDomainGetCPUStats(domain, params, nparams, 0, ncpus, 
 flags);

This will fail if ncpus  128.  You have to do this in a for loop,
grabbing only 128 cpus per iteration.

 +LIBVIRT_END_ALLOW_THREADS;
 +
 +if (i_retval  0) {
 +error = VIR_PY_NONE;
 +goto cleanup;
 +}
 +
 +for (i = 0; i  ncpus; i++) {
 +if (params[i * nparams].type == 0)
 +continue;

Technically, this is only possible if you called virDomainGetCPUStats
with ncpus larger than what the hypervisor already told you to use.  But
I guess it doesn't hurt to leave these two lines in.

 +
 +cpuparams = params[i * nparams];
 +if ((cpu = getPyVirTypedParameter(cpuparams, nparams)) == NULL) {

Here, you want to iterate over the number of returned parameters, not
the number of array slots you passed in.  s/nparams/i_retval/

 +error = NULL;
 +goto cleanup;
 +}
 +if (PyList_Append(ret, cpu)  0) {
 +Py_DECREF(cpu);
 +error = NULL;
 +goto cleanup;
 +}
 +Py_DECREF(cpu);
 +}
 +
 +virTypedParameterArrayClear(params, 

Re: [libvirt] [PATCH v2] python: add virDomainGetCPUStats python binding API

2012-03-16 Thread Eric Blake
On 03/16/2012 11:26 AM, Eric Blake wrote:
 
 +
 +if (!nparams) {
 +if ((cpu = PyDict_New()) == NULL) {
 +error = NULL;
 
 Initialize error to NULL at the front, and you won't have to set it here.
 
 +goto failed;
 +}
 +
 +if (PyList_Append(ret, cpu)  0) {
 +Py_DECREF(cpu);
 +error = NULL;
 +goto failed;
 +}
 +
 +Py_DECREF(cpu);
 +return ret;
 
 So why are we populating the list with a single empty dictionary?
 Shouldn't we instead be populating it with one empty dictionary per cpu?
  That is, this code returns [{}], but if ncpus is 4, it should return
 [{},{},{},{}].

In fact, instead of returning early here, you can just say:

if (nparams) {

// get parameters, via for loop over...

 
 +}
 +sumparams = nparams * ncpus;
 +
 +if (VIR_ALLOC_N(params, sumparams)  0) {
 +error = PyErr_NoMemory();
 +goto failed;
 +}
 +
 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +i_retval = virDomainGetCPUStats(domain, params, nparams, 0, ncpus, 
 flags);
 
 This will fail if ncpus  128.  You have to do this in a for loop,
 grabbing only 128 cpus per iteration.
 
 +LIBVIRT_END_ALLOW_THREADS;
 +
 +if (i_retval  0) {
 +error = VIR_PY_NONE;
 +goto cleanup;
 +}

} else {
i_retval = 0;
}

 +
 +for (i = 0; i  ncpus; i++) {
 +if (params[i * nparams].type == 0)
 +continue;
 
 Technically, this is only possible if you called virDomainGetCPUStats
 with ncpus larger than what the hypervisor already told you to use.  But
 I guess it doesn't hurt to leave these two lines in.

then drop these two lines after all (since in the i_retval==0 case,
params might be NULL and this would be a NULL deref),

 
 +
 +cpuparams = params[i * nparams];

This is just an address computation, so it doesn't matter if params is NULL.

 +if ((cpu = getPyVirTypedParameter(cpuparams, nparams)) == NULL) 
 {
 
 Here, you want to iterate over the number of returned parameters, not
 the number of array slots you passed in.  s/nparams/i_retval/

and this will properly create your empty dictionary, since
getPyVirTypedParameter does the right thing if i_retval is 0.  That way,
your PyList_Append() code can be reused to populate the correct return
value for ncpus regardless of whether nparams was 0.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] Add migration APIs for libxl driver

2012-03-16 Thread Jim Fehlig
Jim Fehlig wrote:
 +static int
 +libxlDomainMigratePerform3(virDomainPtr dom,
 +const char *xmlin ATTRIBUTE_UNUSED,
 +const char *cookiein ATTRIBUTE_UNUSED,
 +int cookieinlen ATTRIBUTE_UNUSED,
 +char **cookieout ATTRIBUTE_UNUSED,
 +int *cookieoutlen ATTRIBUTE_UNUSED,
 +const char *dconnuri ATTRIBUTE_UNUSED,
 +const char *uri,
 +unsigned long flags,
 +const char *dname ATTRIBUTE_UNUSED,
 +unsigned long resource ATTRIBUTE_UNUSED)
 +{
 +libxlDriverPrivatePtr driver = dom-conn-privateData;
 +char *hostname = NULL;
 +int port;
 +char *servname = NULL;
 +struct addrinfo hints, *res;
 +int sockfd = -1;
 +int ret = -1;
 +
 +virCheckFlags(LIBXL_MIGRATION_FLAGS, -1);
 +
 +libxlDriverLock(driver);
 +
 +if (doParseURI(uri, hostname, port))
 +goto cleanup;
 +
 +VIR_DEBUG(hostname = %s, port = %d, hostname, port);
 +
 +if (virAsprintf(servname, %d, port ? port : DEFAULT_MIGRATION_PORT) 
  0) {
 +virReportOOMError();
 +goto cleanup;
 +}
 +
 +if ((sockfd = socket(AF_INET, SOCK_STREAM, 0))  0 ){
 +libxlError(VIR_ERR_OPERATION_FAILED,
 + _(Failed to create socket));
 +goto cleanup;
 +}
 +
 +memset(hints, 0, sizeof(hints));
 +hints.ai_family = AF_INET;
 +hints.ai_socktype = SOCK_STREAM;
 +if (getaddrinfo(hostname, servname, hints, res) || res == NULL) {
 +libxlError(VIR_ERR_OPERATION_FAILED,
 + _(IP address lookup failed));
 +goto cleanup;
 +}
 +
 +if (connect(sockfd, res-ai_addr, res-ai_addrlen)  0) {
 +libxlError(VIR_ERR_OPERATION_FAILED,
 + _(Connect error));
 +goto cleanup;
 +}
 +
 +ret = doMigrateSend(dom, flags, sockfd);
   
 

 Hmm, the entire driver is locked during the migration.  I just noticed
 libxlDomainSave{Flags} also holds the driver lock during save :-(. 
 libxlDomainCoreDump drops the lock during the dump and IMO migration
 should follow the same pattern.
   

After some more testing, following the pattern in libxlDomainCoreDump is
not a good idea as it leads to a deadlock.

# virsh dump dom /var/lib/libvirt/libxl/save/test.dump
 d1. lock driver
 d2. get virDomainObjPtr for domain, acquiring dom obj lock
 d3. unlock driver
 d4. core dump domain
 d5. lock driver
 d6. do post dump stuff
 d7. unlock driver
 d8. unlock dom obj lock

While d4 is in progress, list domains

# virsh list
 l1. get num domains
 l2. lock driver
 l3. call virDomainObjListNumOfDomains, which iterates through domains,
obtaining dom obj lock in process

l3 will block when trying to obtain dom obj lock for the domain being
dumped, and note that it is holding the driver lock.  When d4 is
finished, it will attempt to acquire the driver lock - deadlock.  A
quick fix would be to lock the driver for duration of the dump, similar
to save.  But we really need to prevent locking the driver during these
long running operations.

Question for other libvirt devs:

Many of the libxl driver functions use this pattern
 - lock driver
 - vm = virDomainFindByUUID // acquires dom obj lock
 - unlock driver
 - do stuff
- virDomainObjUnlock

In some cases, do stuff requires obtaining the driver lock (e.g.
removing a domain from driver's domain list), in which case there is
potential for deadlock.  Any suggestions for preventing the deadlock
*and* avoiding locking the driver during long running operations?

Thanks,
Jim

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


Re: [libvirt] [PATCH] Add migration APIs for libxl driver

2012-03-16 Thread Eric Blake
On 03/16/2012 11:53 AM, Jim Fehlig wrote:

 
 Question for other libvirt devs:
 
 Many of the libxl driver functions use this pattern
  - lock driver
  - vm = virDomainFindByUUID // acquires dom obj lock
  - unlock driver
  - do stuff
 - virDomainObjUnlock
 
 In some cases, do stuff requires obtaining the driver lock (e.g.
 removing a domain from driver's domain list), in which case there is
 potential for deadlock.  Any suggestions for preventing the deadlock
 *and* avoiding locking the driver during long running operations?

See src/qemu/THREADS.txt.  That gives the details on how to drop the
driver lock and even the domain object lock, by maintaining a reference
count and job condition on each domain to ensure that others can still
use the driver, just not for the domain whose job is still occupied.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] virsh: trim aliases from -h output

2012-03-16 Thread Eric Blake
Commit af3f9aab taught 'virsh help' to ignore command aliases,
but forgot 'virsh -h'.

* tools/virsh.c (vshUsage): Handle aliases.
---
 tools/virsh.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 19f9bbe..e48c56a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -19721,8 +19721,11 @@ vshUsage(void)
 fprintf(stdout, _( %s (help keyword '%s')\n), grp-name, 
grp-keyword);

 for (cmd = grp-commands; cmd-name; cmd++)
+if (cmd-flags  VSH_CMD_FLAG_ALIAS)
+continue;
 fprintf(stdout,
-%-30s %s\n, cmd-name, _(vshCmddefGetInfo(cmd, 
help)));
+%-30s %s\n, cmd-name,
+_(vshCmddefGetInfo(cmd, help)));

 fprintf(stdout, \n);
 }
-- 
1.7.7.6

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


[libvirt] QEMU was not selected for Google Summer of Code this year

2012-03-16 Thread Stefan Hajnoczi
Sad news - QEMU was not accepted for Google Summer of Code 2012.

Students can consider other organizations in the accepted
organizations list here:

http://www.google-melange.com/gsoc/accepted_orgs/google/gsoc2012

The list is currently not complete but should be finalized over the
next few days as organizations complete their profiles.

Students and mentors who wanted to participate with QEMU will be
disappointed.  I am too but there are many factors that organizations
are considered against, we have not received information why QEMU was
rejected this year.

QEMU will try again for sure next year because Google Summer of Code
is a great program both for students and for QEMU.

Stefan

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


Re: [libvirt] [Qemu-devel] QEMU was not selected for Google Summer of Code this year

2012-03-16 Thread Natalia Portillo
Really sad news :(

On 16/03/2012, at 19:29, Stefan Hajnoczi wrote:

 Sad news - QEMU was not accepted for Google Summer of Code 2012.
 
 Students can consider other organizations in the accepted
 organizations list here:
 
 http://www.google-melange.com/gsoc/accepted_orgs/google/gsoc2012
 
 The list is currently not complete but should be finalized over the
 next few days as organizations complete their profiles.
 
 Students and mentors who wanted to participate with QEMU will be
 disappointed.  I am too but there are many factors that organizations
 are considered against, we have not received information why QEMU was
 rejected this year.

I've noted this year there are half the approved organizations than last year...

 QEMU will try again for sure next year because Google Summer of Code
 is a great program both for students and for QEMU.
 
 Stefan
 

Natalia Portllo

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


[libvirt] [PATCH] snapshot: make quiesce a bit safer

2012-03-16 Thread Eric Blake
If a guest is paused, we were silently ignoring the quiesce flag,
which results in unclean snapshots, contrary to the intent of the
flag.  Since we can't quiesce without guest agent support, we should
instead fail if the guest is not running.

Meanwhile, if we attempt a quiesce command, but the guest agent
doesn't respond, and we time out, we may have left the command
pending on the guest's queue, and when the guest resumes parsing
commands, it will freeze even though our command is no longer
around to issue a thaw.  To be safe, we must _always_ pair every
quiesce call with a counterpart thaw, even if the quiesce call
failed due to a timeout, so that if a guest wakes up and starts
processing a command backlog, it will not get stuck in a frozen
state.

* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive):
Always issue thaw after a quiesce, even if quiesce failed.
(qemuDomainSnapshotFSThaw): Add a parameter.
---

See also: https://bugzilla.redhat.com/show_bug.cgi?id=804210
https://www.redhat.com/archives/libvir-list/2012-March/msg00708.html

 src/qemu/qemu_driver.c |   51 +--
 1 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2c467ab..13ca92f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9584,24 +9584,36 @@ qemuDomainSnapshotFSFreeze(struct qemud_driver *driver,

 static int
 qemuDomainSnapshotFSThaw(struct qemud_driver *driver,
- virDomainObjPtr vm) {
+ virDomainObjPtr vm, bool report)
+{
 qemuDomainObjPrivatePtr priv = vm-privateData;
 int thawed;
+virErrorPtr err = NULL;

 if (priv-agentError) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
-_(QEMU guest agent is not 
-  available due to an error));
+if (report)
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(QEMU guest agent is not 
+  available due to an error));
 return -1;
 }
 if (!priv-agent) {
-qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
-_(QEMU guest agent is not configured));
+if (report)
+qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+_(QEMU guest agent is not configured));
 return -1;
 }

 qemuDomainObjEnterAgent(driver, vm);
+if (!report)
+err = virGetLastError();
 thawed = qemuAgentFSThaw(priv-agent);
+if (!report) {
+if (err)
+virResetError(err);
+else
+virResetLastError();
+}
 qemuDomainObjExitAgent(driver, vm);

 return thawed;
@@ -9907,6 +9919,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
 int ret = -1;
 int i;
 bool persist = false;
+int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */

 if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY)  0)
 return -1;
@@ -9917,14 +9930,21 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
 goto endjob;
 }

-
-if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
-if ((flags  VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) 
-(qemuDomainSnapshotFSFreeze(driver, vm)  0)) {
+/* If quiesce was requested, then issue a freeze command, and a
+ * counterpart thaw command, no matter what.  The command will
+ * fail if the guest is paused or the guest agent is not
+ * running.  */
+if (flags  VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) {
+if (qemuDomainSnapshotFSFreeze(driver, vm)  0) {
 /* helper reported the error */
+thaw = -1;
 goto endjob;
+} else {
+thaw = 1;
 }
+}

+if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
 /* In qemu, snapshot_blkdev on a single disk will pause cpus,
  * but this confuses libvirt since notifications are not given
  * when qemu resumes.  And for multiple disks, libvirt must
@@ -10001,11 +10021,6 @@ cleanup:
 _(resuming after snapshot failed));
 goto endjob;
 }
-
-if ((flags  VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) 
-qemuDomainSnapshotFSThaw(driver, vm)  0) {
-/* helper reported the error */
-}
 }

 if (vm) {
@@ -10016,6 +10031,12 @@ cleanup:
 }

 endjob:
+if (vm  thaw != 0 
+qemuDomainSnapshotFSThaw(driver, vm, thaw  0)  0) {
+/* helper reported the error, if it was needed */
+if (thaw  0)
+ret = -1;
+}
 if (vm  (qemuDomainObjEndJob(driver, vm) == 0)) {
 /* Only possible if a transient vm quit while our locks were down,
  * in which case we don't want to save snapshot metadata.  */
-- 
1.7.7.6

--
libvir-list mailing list
libvir-list@redhat.com

Re: [libvirt] heisenbug in command.c

2012-03-16 Thread Jim Paris
Serge Hallyn wrote:
 On 03/16/2012 11:50 AM, Eric Blake wrote:
 On 03/16/2012 10:36 AM, Serge Hallyn wrote:
 Hi,
 
 It seems I've run into quite the heisenbug, reported at
 https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/922628
 
 It manifests itself as virPidWait returning status=4 for iptables (which
 should never exit with status=4).
 
 Maybe iptables isn't documented as exiting with $? of 4, but that's what
 is happening.  The libvirt code in question is quite clear that it
 grabbed an accurate exit status from the child process.
 
 
 Well, yes.  I figured that either (1) iptables actually got -EINTR
 from the kernel and passed that along as its exit code, or (2)
 something went wrong with memory being overwritten in libvirt,
 however unlikely. Stranger things have happened.  If (1), I was
 wondering if it was being ignored on purpose.

Why do you bring up EINTR at all?  Just because EINTR is 4?  That
seems very much unrelated.

This is from iptables:

enum xtables_exittype {
OTHER_PROBLEM = 1,
PARAMETER_PROBLEM,
VERSION_PROBLEM,
RESOURCE_PROBLEM,
XTF_ONLY_ONCE,
XTF_NO_INVERT,
XTF_BAD_VALUE,
XTF_ONE_ACTION,
};

So it looks like iptables is returning RESOURCE_PROBLEM (which could
explain why it's intermittent).

-jim

  ret = virPidWait(cmd-pid, exitstatus ? exitstatus :status);
  if (ret == 0) {
  cmd-pid = -1;
  cmd-reap = false;
  if (status) {
  char *str = virCommandToString(cmd);
  char *st = virCommandTranslateStatus(status);
  virCommandError(VIR_ERR_INTERNAL_ERROR,
  _(Child process (%s) status unexpected: %s),
  str ? str : cmd-args[0], NULLSTR(st));
 
   But it's only been seen on two (very
 different) machines, and the slightest shifting of the winds makes it go
 away.  Given how sneaky this bug appears to be, there's a slight
 temptation to have iptablesAddRemoveRule pass in a int* for status and
 better deal with the -EINTR.  But I fear that might be papering over a
 worse race.
 
 I don't follow how you think there is a -EINTR being encountered in
 libvirt.
 
 Yeah I don't really either.
 
  I think you'd be better off investigating why iptables really
 is exiting with status 4.
 
 Well, given what EINTR means, shouldn't src/util/iptables.c re-try
 the command if it gets that?
 
 Anyway I'll keep digging, but was wondering if anyone else has seen this.
 
 -serge
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

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


Re: [libvirt] heisenbug in command.c

2012-03-16 Thread Serge Hallyn

On 03/16/2012 03:52 PM, Jim Paris wrote:

Serge Hallyn wrote:

On 03/16/2012 11:50 AM, Eric Blake wrote:

On 03/16/2012 10:36 AM, Serge Hallyn wrote:

Hi,

It seems I've run into quite the heisenbug, reported at
https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/922628

It manifests itself as virPidWait returning status=4 for iptables (which
should never exit with status=4).


Maybe iptables isn't documented as exiting with $? of 4, but that's what
is happening.  The libvirt code in question is quite clear that it
grabbed an accurate exit status from the child process.



Well, yes.  I figured that either (1) iptables actually got -EINTR
from the kernel and passed that along as its exit code, or (2)
something went wrong with memory being overwritten in libvirt,
however unlikely. Stranger things have happened.  If (1), I was
wondering if it was being ignored on purpose.


Why do you bring up EINTR at all?  Just because EINTR is 4?


Yup.

  That

seems very much unrelated.


I didn't think so, but looks like you're right :)


This is from iptables:

enum xtables_exittype {
 OTHER_PROBLEM = 1,
 PARAMETER_PROBLEM,
 VERSION_PROBLEM,
 RESOURCE_PROBLEM,
 XTF_ONLY_ONCE,
 XTF_NO_INVERT,
 XTF_BAD_VALUE,
 XTF_ONE_ACTION,
};

So it looks like iptables is returning RESOURCE_PROBLEM (which could
explain why it's intermittent).


That makes a lot more sense then, yes.  When scripting a bunch of 
parallel iptables rules adds (followed by waits), I do once in awhile get


iptables: Resource temporarily unavailable.

(and sometimes a -EINVAL message)

though 'wait' still says status was 0.  I've never gotten 4.  The next 
run then succeeds.


This still however sounds like src/util/iptables.c might ought to re-run 
the command if it gets 4.


thanks,
-serge

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


[libvirt] [PATCH 0/3] atomic snapshots, round 1

2012-03-16 Thread Eric Blake
I figured it's better to post small patch series for incremental
review than to slam the list with a 20 patch series.  I'm in the
middle of coding up my RFC for interaction with the new qemu 1.1
'transaction' monitor command, and this is the first step.

[1] https://www.redhat.com/archives/libvir-list/2012-March/msg00578.html

To avoid a merge conflict, this patch [2] needs to be applied first.

[2] https://www.redhat.com/archives/libvir-list/2012-March/msg00744.html

Eric Blake (3):
  snapshot: add qemu capability for 'transaction' command
  snapshot: add atomic create flag
  snapshot: rudimentary qemu support for atomic disk snapshot

 include/libvirt/libvirt.h.in |2 ++
 src/libvirt.c|   21 ++---
 src/qemu/qemu_capabilities.c |1 +
 src/qemu/qemu_capabilities.h |1 +
 src/qemu/qemu_driver.c   |   40 +++-
 src/qemu/qemu_monitor_json.c |3 +++
 tools/virsh.c|6 ++
 tools/virsh.pod  |   16 ++--
 8 files changed, 68 insertions(+), 22 deletions(-)

-- 
1.7.7.6

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


[libvirt] [PATCH 2/3] snapshot: add atomic create flag

2012-03-16 Thread Eric Blake
Right now, it is appallingly easy to cause qemu disk snapshots
to alter a domain then fail; for example, by requesting a two-disk
snapshot where the second disk name resides on read-only storage.
In this failure scenario, libvirt reports failure, but modifies
the live domain XML in-place to record that the first disk snapshot
was taken; and places a difficult burden on the management app
to grab the XML and reparse it to see which disks, if any, were
altered by the partial snapshot.

This patch adds a new flag where implementations can request that
the hypervisor make snapshots atomically; either no changes to
XML occur, or all disks were altered as a group.  If you request
the flag, you either get outright failure up front, or you take
advantage of hypervisor abilities to make an atomic snapshot. Of
course, drivers should prefer the atomic means even without the
flag explicitly requested.

There's no way to make snapshots 100% bulletproof - even if the
hypervisor does it perfectly atomic, we could run out of memory
during the followup tasks of updating our in-memory XML, and report
a failure.  However, these sorts of catastrophic failures are rare
and unlikely, and it is still nicer to know that either all
snapshots happened or none of them, as that is an easier state to
recover from.

* include/libvirt/libvirt.h.in
(VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC): New flag.
* src/libvirt.c (virDomainSnapshotCreateXML): Document it.
* tools/virsh.c (cmdSnapshotCreate, cmdSnapshotCreateAs): Expose it.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Document
it.
---
 include/libvirt/libvirt.h.in |2 ++
 src/libvirt.c|   21 ++---
 tools/virsh.c|6 ++
 tools/virsh.pod  |   16 ++--
 4 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 7d41642..4566580 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3289,6 +3289,8 @@ typedef enum {
   quiesce all mounted
   file systems within
   the domain */
+VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC  = (1  7), /* atomically avoid
+  partial changes */
 } virDomainSnapshotCreateFlags;

 /* Take a snapshot of the current VM state */
diff --git a/src/libvirt.c b/src/libvirt.c
index 7f8d42c..48d08c4 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17119,14 +17119,21 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr 
snapshot)
  * @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, then existing
  * destination files are instead truncated and reused.
  *
- * Returns an (opaque) virDomainSnapshotPtr on success, NULL on failure.
  * Be aware that although libvirt prefers to report errors up front with
- * no other effect, there are certain types of failures where a failure
- * can occur even though the guest configuration was changed (for
- * example, if a disk snapshot request over two disks only fails on the
- * second disk, leaving the first disk altered); so after getting a NULL
- * return, it can be wise to use virDomainGetXMLDesc() to determine if
- * any partial changes occurred.
+ * no other effect, some hypervisors have certain types of failures where
+ * the overall command can easily fail even though the guest configuration
+ * was partially altered (for example, if a disk snapshot request for two
+ * disks fails on the second disk, but the first disk alteration cannot be
+ * rolled back).  If this API call fails, it is therefore normally
+ * necessary to follow up with virDomainGetXMLDesc() and check each disk
+ * to determine if any partial changes occurred.  However, if @flags
+ * contains VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC, then libvirt guarantees
+ * that this command will not alter any disks unless the entire set of
+ * changes can be done atomically, making failure recovery simpler (note
+ * that it is still possible to fail after disks have changed, but only
+ * in the much rarer cases of running out of memory or disk space).
+ *
+ * Returns an (opaque) virDomainSnapshotPtr on success, NULL on failure.
  */
 virDomainSnapshotPtr
 virDomainSnapshotCreateXML(virDomainPtr domain,
diff --git a/tools/virsh.c b/tools/virsh.c
index e48c56a..980e206 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -15715,6 +15715,7 @@ static const vshCmdOptDef opts_snapshot_create[] = {
 {disk-only, VSH_OT_BOOL, 0, N_(capture disk state but not vm state)},
 {reuse-external, VSH_OT_BOOL, 0, N_(reuse any existing external 
files)},
 {quiesce, VSH_OT_BOOL, 0, N_(quiesce guest's file systems)},
+{atomic, VSH_OT_BOOL, 0, N_(require atomic operation)},
 {NULL, 0, 0, NULL}
 };

@@ -15741,6 +15742,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
 

[libvirt] [PATCH 1/3] snapshot: add qemu capability for 'transaction' command

2012-03-16 Thread Eric Blake
We need a capability bit to gracefully error out if some of the
additions in future patches can't be implemented by the running qemu.

* src/qemu/qemu_capabilities.h (QEMU_CAPS_TRANSACTION): New cap.
* src/qemu/qemu_capabilities.c (qemuCaps): Name it.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONCheckCommands): Set
it.
---

I haven't yet decided if I need to check for 'block-mirror' as an
independent capability, but that can be a later patch.

 src/qemu/qemu_capabilities.c |1 +
 src/qemu/qemu_capabilities.h |1 +
 src/qemu/qemu_monitor_json.c |3 +++
 3 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ace5011..0e09d6d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -155,6 +155,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
   system_wakeup,
   scsi-disk.channel,
   scsi-block,
+  transaction,
 );

 struct qemu_feature_flags {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 62b4270..78cdbe0 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -123,6 +123,7 @@ enum qemuCapsFlags {
 QEMU_CAPS_WAKEUP = 86, /* system_wakeup monitor command */
 QEMU_CAPS_SCSI_DISK_CHANNEL  = 87, /* Is scsi-disk.channel available? */
 QEMU_CAPS_SCSI_BLOCK = 88, /* -device scsi-block */
+QEMU_CAPS_TRANSACTION= 89, /* transaction monitor command */

 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index ce68e69..4dd6924 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -884,6 +884,9 @@ qemuMonitorJSONCheckCommands(qemuMonitorPtr mon,

 if (STREQ(name, system_wakeup))
 qemuCapsSet(qemuCaps, QEMU_CAPS_WAKEUP);
+
+if (STREQ(name, transaction))
+qemuCapsSet(qemuCaps, QEMU_CAPS_TRANSACTION);
 }

 ret = 0;
-- 
1.7.7.6

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


[libvirt] [PATCH 3/3] snapshot: rudimentary qemu support for atomic disk snapshot

2012-03-16 Thread Eric Blake
Taking an external snapshot of just one disk is atomic, without having
to pause and resume the VM.  This also paves the way for later patches
to interact with the new qemu 'transaction' monitor command.

The various scenarios when requesting atomic are:
online, 1 disk, old qemu - safe, allowed by this patch
online, more than 1 disk, old qemu - failure, this patch
offline snapshot - safe, once a future patch implements offline disk snapshot
online, 1 or more disks, new qemu - safe, once future patch uses transaction

* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Support
new flag for single-disk setups.
(qemuDomainSnapshotDiskPrepare): Check for atomic here.
(qemuDomainSnapshotCreateDiskActive): Skip pausing the VM when
atomic supported.
---
 src/qemu/qemu_driver.c |   40 +++-
 1 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 13ca92f..a46ce10 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9712,13 +9712,16 @@ endjob:

 static int
 qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
-  bool allow_reuse)
+  unsigned int *flags)
 {
 int ret = -1;
 int i;
 bool found = false;
 bool active = virDomainObjIsActive(vm);
 struct stat st;
+bool allow_reuse = (*flags  VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
+bool atomic = (*flags  VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) != 0;
+int external = 0;

 for (i = 0; i  def-ndisks; i++) {
 virDomainSnapshotDiskDefPtr disk = def-disks[i];
@@ -9773,6 +9776,7 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, 
virDomainSnapshotDefPtr def,
 goto cleanup;
 }
 found = true;
+external++;
 break;

 case VIR_DOMAIN_DISK_SNAPSHOT_NO:
@@ -9792,6 +9796,13 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, 
virDomainSnapshotDefPtr def,
   selected for snapshot));
 goto cleanup;
 }
+if (atomic  external  1) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+_(atomic snapshot of multiple disks is unsupported));
+goto cleanup;
+} else if (external == 1) {
+*flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
+}

 ret = 0;

@@ -9920,6 +9931,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
 int i;
 bool persist = false;
 int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */
+bool atomic = (flags  VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) != 0;

 if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY)  0)
 return -1;
@@ -9944,14 +9956,14 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
 }
 }

-if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
-/* In qemu, snapshot_blkdev on a single disk will pause cpus,
- * but this confuses libvirt since notifications are not given
- * when qemu resumes.  And for multiple disks, libvirt must
- * pause externally to get all snapshots to be at the same
- * point in time.  For simplicitly, we always pause ourselves
- * rather than relying on qemu doing pause.
- */
+/* For multiple disks, libvirt must pause externally to get all
+ * snapshots to be at the same point in time, unless qemu supports
+ * transactions.  For a single disk, snapshot is atomic without
+ * requiring a pause.  Thanks to qemuDomainSnapshotDiskPrepare, if
+ * we got to this point, the atomic flag now says whether we need
+ * to pause, and a capability bit says whether to use transaction.
+ */
+if (!atomic  virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
 if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE,
 QEMU_ASYNC_JOB_NONE)  0)
 goto cleanup;
@@ -10069,7 +10081,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
   VIR_DOMAIN_SNAPSHOT_CREATE_HALT |
   VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY |
   VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT |
-  VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, NULL);
+  VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE |
+  VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC, NULL);

 if ((flags  VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) 
 !(flags  VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)) {
@@ -10211,14 +10224,15 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
 goto cleanup;

 if (flags  VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
-bool allow_reuse;
+qemuDomainObjPrivatePtr priv = vm-privateData;

-allow_reuse = (flags  VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
 if (virDomainSnapshotAlignDisks(def,
 VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL,
 

Re: [libvirt] [Qemu-devel] QEMU was not selected for Google Summer of Code this year

2012-03-16 Thread Stefan Hajnoczi
On Fri, Mar 16, 2012 at 7:44 PM, Natalia Portillo clau...@claunia.com wrote:
 Really sad news :(

 On 16/03/2012, at 19:29, Stefan Hajnoczi wrote:

 Sad news - QEMU was not accepted for Google Summer of Code 2012.

 Students can consider other organizations in the accepted
 organizations list here:

 http://www.google-melange.com/gsoc/accepted_orgs/google/gsoc2012

 The list is currently not complete but should be finalized over the
 next few days as organizations complete their profiles.

 Students and mentors who wanted to participate with QEMU will be
 disappointed.  I am too but there are many factors that organizations
 are considered against, we have not received information why QEMU was
 rejected this year.

 I've noted this year there are half the approved organizations than last 
 year...

The list of accepted orgs is not complete yet, more will be displayed
as they fill out their details in the http://google-melange.com/ web
app.

Stefan

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


Re: [libvirt] [PATCH] Increased upper limit on lists of pool names

2012-03-16 Thread Jesse Cook
On Fri, Mar 16, 2012 at 7:35 AM, Eric Blake ebl...@redhat.com wrote:
 On 03/15/2012 08:26 PM, Jesse Cook wrote:

 v0.9.10 client did not want to play nicely with the v0.9.10 server via
 qemu+ssh.  I got frustrated and just tried running the test from a
 client running an older version of libvirt.  When connecting to
 v0.9.10, it behaved the same way the pre-patched did in my cover
 letter.  I don't have full test results because of the communication
 errors I was getting. I'll try to either figure it out tomorrow or
 just use the older version as the client (pre-patch and patch).

 Since qemu already uses rpc mechanism, I've found it easy to
 mix-and-match by doing this in two terminals both logged in as root, and
 both located in a libvirt.git checkout directory (if you don't mind
 building as root; if you do mind building as root, then have three,
 where the third is a regular user running as non-root to do the builds):

 setup:
 1# git clone libvirt ...
 1# cd libvirt
 1# ./autobuild.sh --system
 1# apply patches I'm interested in testing
 1# make
 2# cd libvirt

 old libvirtd server, old virsh client:
 1# service libvirtd start
 2# virsh ...
 1# service libvirtd stop

 old libvirtd, new virsh
 1# service libvirtd start
 2# tools/virsh ...
 1# service libvirtd stop

 new libvirtd, old virsh
 1# daemon/libvirtd
 2# virsh ...
 1# Ctrl-C

 new libvirtd, new virsh
 1# daemon/libvirtd
 2# tools/virsh ...
 1# Ctrl-C

 That gives me testing of all four protocol combinations, assuming that
 the installed version is a current release without my patch, and my
 just-built in-tree version has my patch.  No need to complicate protocol
 testing with multi-host qemu+ssh:// stuff.

 --
 Eric Blake   ebl...@redhat.com+1-919-301-3266
 Libvirt virtualization library http://libvirt.org


Eric,

Thanks.  v0.9.10 was giving me a lot of trouble, so I just compiled two
versions of the test code in the cover letter.  One uses the patched libs
and one the unchanged libs.

N.B. I compiled the patched version using the following change due to the
issues with using 65536:

/* Upper limit on lists of storage pool names. */
-const REMOTE_STORAGE_POOL_NAME_LIST_MAX = 256;
+const REMOTE_STORAGE_POOL_NAME_LIST_MAX = 7084;

Results:

With the patched version of libvirt installed and unpatched libvirtd
running:

for i in `ls -1 /etc/libvirt/storage/*`; do virsh pool-start $(basename $i
.xml); done /dev/null
~/pool-test-unpatched
Before virConnectNumOfStoragePools
After virConnectNumOfStoragePools
Before virConnectListStoragePools
libvir: Remote error : too many remote undefineds: 407  256
After virConnectListStoragePools
Active: -1
Before virConnectNumOfDefinedStoragePools
After virConnectNumOfDefinedStoragePools
Inactive: -1
~/pool-test-patched
Before virConnectNumOfStoragePools
After virConnectNumOfStoragePools
Before virConnectListStoragePools
After virConnectListStoragePools
Active: 407
Before virConnectNumOfDefinedStoragePools
After virConnectNumOfDefinedStoragePools
Inactive: -1
for i in `ls -1 /etc/libvirt/storage/*`; do virsh pool-destroy $(basename
$i .xml); done /dev/null
~/pool-test-unpatched
Before virConnectNumOfStoragePools
After virConnectNumOfStoragePools
Active: -1
Before virConnectNumOfDefinedStoragePools
After virConnectNumOfDefinedStoragePools
Before virConnectListDefinedStoragePools
libvir: Remote error : too many remote undefineds: 407  256
After virConnectListDefinedStoragePools
Inactive: -1
~/pool-test-patched
Before virConnectNumOfStoragePools
After virConnectNumOfStoragePools
Active: -1
Before virConnectNumOfDefinedStoragePools
After virConnectNumOfDefinedStoragePools
Before virConnectListDefinedStoragePools
After virConnectListDefinedStoragePools
Inactive: 407


With the unchanged version of libvirt installed and unpatched libvirtd
running:

for i in `ls -1 /etc/libvirt/storage/*`; do virsh pool-start $(basename $i
.xml); done /dev/null
~/pool-test-unpatched
Before virConnectNumOfStoragePools
After virConnectNumOfStoragePools
Before virConnectListStoragePools
libvir: Remote error : too many remote undefineds: 407  256
After virConnectListStoragePools
Active: -1
Before virConnectNumOfDefinedStoragePools
After virConnectNumOfDefinedStoragePools
Inactive: -1
~/pool-test-patched
Before virConnectNumOfStoragePools
After virConnectNumOfStoragePools
Before virConnectListStoragePools
libvir: RPC error : internal error maxnames 
REMOTE_STORAGE_POOL_NAME_LIST_MAX
After virConnectListStoragePools
Active: -1
Before virConnectNumOfDefinedStoragePools
After virConnectNumOfDefinedStoragePools
Inactive: -1
for i in `ls -1 /etc/libvirt/storage/*`; do virsh pool-destroy $(basename
$i .xml); done /dev/null
~/pool-test-unpatched
Before virConnectNumOfStoragePools
After virConnectNumOfStoragePools
Active: -1
Before virConnectNumOfDefinedStoragePools
After virConnectNumOfDefinedStoragePools
Before virConnectListDefinedStoragePools
libvir: Remote error : too many remote undefineds: 407  256
After virConnectListDefinedStoragePools

Re: [libvirt] [Qemu-devel] QEMU was not selected for Google Summer of Code this year

2012-03-16 Thread Natalia Portillo
QEMU hosted on Haiku would be interesting.

On 16/03/2012, at 22:30, Stefan Hajnoczi wrote:

 On Fri, Mar 16, 2012 at 7:44 PM, Natalia Portillo clau...@claunia.com wrote:
 Really sad news :(
 
 On 16/03/2012, at 19:29, Stefan Hajnoczi wrote:
 
 Sad news - QEMU was not accepted for Google Summer of Code 2012.
 
 Students can consider other organizations in the accepted
 organizations list here:
 
 http://www.google-melange.com/gsoc/accepted_orgs/google/gsoc2012
 
 The list is currently not complete but should be finalized over the
 next few days as organizations complete their profiles.
 
 Students and mentors who wanted to participate with QEMU will be
 disappointed.  I am too but there are many factors that organizations
 are considered against, we have not received information why QEMU was
 rejected this year.
 
 I've noted this year there are half the approved organizations than last 
 year...
 
 The list of accepted orgs is not complete yet, more will be displayed
 as they fill out their details in the http://google-melange.com/ web
 app.
 
 Stefan


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


Re: [libvirt] [PATCH] virsh: trim aliases from -h output

2012-03-16 Thread Osier Yang

On 2012年03月17日 03:23, Eric Blake wrote:

Commit af3f9aab taught 'virsh help' to ignore command aliases,
but forgot 'virsh -h'.

* tools/virsh.c (vshUsage): Handle aliases.
---
  tools/virsh.c |5 -
  1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 19f9bbe..e48c56a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -19721,8 +19721,11 @@ vshUsage(void)
  fprintf(stdout, _( %s (help keyword '%s')\n), grp-name, 
grp-keyword);

  for (cmd = grp-commands; cmd-name; cmd++)
+if (cmd-flags  VSH_CMD_FLAG_ALIAS)
+continue;
  fprintf(stdout,
-%-30s %s\n, cmd-name, _(vshCmddefGetInfo(cmd, 
help)));
+%-30s %s\n, cmd-name,
+_(vshCmddefGetInfo(cmd, help)));

  fprintf(stdout, \n);
  }


ACK

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

Re: [libvirt] [Qemu-devel] QEMU was not selected for Google Summer of Code this year

2012-03-16 Thread Chris Wright
* Natalia Portillo (clau...@claunia.com) wrote:
 QEMU hosted on Haiku would be interesting.

The fun of Haiku
especially when it is
hosting QEMU

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


Re: [libvirt] [PATCH] virsh: trim aliases from -h output

2012-03-16 Thread Eric Blake
On 03/16/2012 09:24 PM, Osier Yang wrote:
 On 2012年03月17日 03:23, Eric Blake wrote:
 Commit af3f9aab taught 'virsh help' to ignore command aliases,
 but forgot 'virsh -h'.

 * tools/virsh.c (vshUsage): Handle aliases.
 ---
   tools/virsh.c |5 -
   1 files changed, 4 insertions(+), 1 deletions(-)

 diff --git a/tools/virsh.c b/tools/virsh.c
 index 19f9bbe..e48c56a 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -19721,8 +19721,11 @@ vshUsage(void)
   fprintf(stdout, _( %s (help keyword '%s')\n), grp-name,
 grp-keyword);

   for (cmd = grp-commands; cmd-name; cmd++)
 +if (cmd-flags  VSH_CMD_FLAG_ALIAS)
 +continue;
   fprintf(stdout,
 -%-30s %s\n, cmd-name,
 _(vshCmddefGetInfo(cmd, help)));
 +%-30s %s\n, cmd-name,
 +_(vshCmddefGetInfo(cmd, help)));


Oh my - I completely missed the {} (emailed the wrong version).

   fprintf(stdout, \n);
   }
 
 ACK

I've pushed the corrected patch.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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