Re: [libvirt] libvirt 0.5.0 and KVM migration

2008-12-02 Thread Mickaël Canévet
On Tue, 2008-12-02 at 09:11 +0100, Chris Lalancette wrote:
 Mickaël Canévet wrote:
  Thanks for your quick answer.
  
  I forgot to restart libvirt on one node.
  
  Now I have an other error message:
  
  libvir: QEMU error : operation failed: failed to start listening VM
  
  Is there any kernel limitation also ? will it work with my distro 2.6.26
  kernel with kvm-79 userspace component or do I have to have kvm-79
  kernel module also, or even kernel then 2.6.26 ?
  
  In other (debian) words, will an apt-get update kvm -t
  experimental (which provides kvm-79) be enough on my lenny (I would
  like to limit unstable or experimental packages) ?
 
 Just an updated kvm-79 userspace *should* work; for instance, in oVirt, we are
 running a kvm-79 userspace equivalent on a 2.6.27 kernel, which is slightly
 older.  In general, a new kvm userspace should work on an older kernel, but it
 is not a heavily tested configuration, so your results might vary.
 
OK,

I updated both userspace and kernel module to kvm-79.
Now migration begins, but:
- On the source node, the command does not returns and an strace shows
that it does nothing (wait4(9709, )
- On the destination node, I can connect threw the VNC display, but
there seams to have a strange issue with I/O (lot of messages on
console; sd 0:0:0:0: rejecting I/O to offline device). FYI my VM disk is
a DRBD over LVM as dual primary.
- From another PC, I lose a few pings during migration.

Are my I/0 errors comes from my backend storage (DRBD) ?


-- 
___
Mickaël Canévet.
European Molecular Biology Laboratory (EMBL)
Grenoble Outstation. FRANCE
___


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


Re: [libvirt] PATCH: 5/28: Stub out node device APIs in test driver

2008-12-02 Thread Daniel Veillard
On Sun, Nov 30, 2008 at 11:29:25PM +, Daniel P. Berrange wrote:
 When adding the node device APIs we never added a stub for the test driver,
 so  the test driver was connecting to the daemon for node device APIS,
 which rather defeats the purpose of the test driver. This patch doesn't
 implement the node device APIs, just stubs out the open/close methods
 so we don't call into the daemon. 

   heh :-) +1

Daniel

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

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


Re: [libvirt] PATCH: 12/28: Thread safety for UML driver

2008-12-02 Thread Daniel Veillard
On Sun, Nov 30, 2008 at 11:53:17PM +, Daniel P. Berrange wrote:
 This patch makes the UML driver thread safe

[...]
 @@ -1395,21 +1485,27 @@ static int umlListDefinedDomains(virConn
  struct uml_driver *driver = conn-privateData;
  int got = 0, i;
  
 +umlDriverLock(driver);
  for (i = 0 ; i  driver-domains.count  got  nnames ; i++) {
 +virDomainObjLock(driver-domains.objs[i]);
  if (!virDomainIsActive(driver-domains.objs[i])) {
  if (!(names[got++] = 
 strdup(driver-domains.objs[i]-def-name))) {
  umlReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
   %s, _(failed to allocate space for VM 
 name string));
 +virDomainObjUnlock(driver-domains.objs[i]);
  goto cleanup;
  }
  }
 +virDomainObjUnlock(driver-domains.objs[i]);
  }
 +umlDriverUnlock(driver);
  
  return got;
  
   cleanup:

  since this is an error code path, I would rather change the label to
error: ...

  Okay otherwise nothing spotted, +1

Daniel

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

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


Re: [libvirt] PATCH: 12/28: Thread safety for UML driver

2008-12-02 Thread Daniel P. Berrange
On Tue, Dec 02, 2008 at 12:05:10PM +0100, Daniel Veillard wrote:
 On Sun, Nov 30, 2008 at 11:53:17PM +, Daniel P. Berrange wrote:
  This patch makes the UML driver thread safe
 
 [...]
  @@ -1395,21 +1485,27 @@ static int umlListDefinedDomains(virConn
   struct uml_driver *driver = conn-privateData;
   int got = 0, i;
   
  +umlDriverLock(driver);
   for (i = 0 ; i  driver-domains.count  got  nnames ; i++) {
  +virDomainObjLock(driver-domains.objs[i]);
   if (!virDomainIsActive(driver-domains.objs[i])) {
   if (!(names[got++] = 
  strdup(driver-domains.objs[i]-def-name))) {
   umlReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
%s, _(failed to allocate space for VM 
  name string));
  +virDomainObjUnlock(driver-domains.objs[i]);
   goto cleanup;
   }
   }
  +virDomainObjUnlock(driver-domains.objs[i]);
   }
  +umlDriverUnlock(driver);
   
   return got;
   
cleanup:
 
   since this is an error code path, I would rather change the label to
 error: ...

That's a good idea - we should define a standard naming for this usage

  'cleanup' to be used where error  normal conditions shared the same
exit path of a method.

  'error' to be used where error path is completely separate from the
  normal exit path.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] Concerning umlInotifyEvent

2008-12-02 Thread Daniel P. Berrange
On Sun, Nov 30, 2008 at 08:10:11PM +, Ron Yorston wrote:
 Daniel P. Berrange [EMAIL PROTECTED] wrote:
 That's a symptom of a bug earlier on. Try this patch:
 
 Thanks, Daniel, that seems to have fixed it.

Thanks for confirming - I've comitted this patch.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] [PATCH] post-release patch for symbols

2008-12-02 Thread Daniel P. Berrange
On Tue, Dec 02, 2008 at 11:30:52AM +0100, Daniel Veillard wrote:
   Assuming we release a 0.5.1 soon. Maybe the code with threading
 should be on a 0.6.0 considering the amount of changes though.

Better to put it inside comments for now, because we don't really want 
an empty linker version section to be picked up unless we actually add 
new  public symbols in this release. 

 Index: src/libvirt_sym.version.in
 ===
 RCS file: /data/cvs/libxen/src/libvirt_sym.version.in,v
 retrieving revision 1.6
 diff -u -r1.6 libvirt_sym.version.in
 --- src/libvirt_sym.version.in21 Nov 2008 12:27:11 -  1.6
 +++ src/libvirt_sym.version.in2 Dec 2008 10:29:08 -
 @@ -249,6 +249,8 @@
  
  } LIBVIRT_0.4.5;
  
 +LIBVIRT_0.5.1 {
 +} LIBVIRT_0.5.0;
  /*  define new API here using predicted next version number  */


Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets

2008-12-02 Thread Daniel P. Berrange
On Mon, Dec 01, 2008 at 12:23:12PM +0100, Gerd v. Egidy wrote:
 Hi Daniel,
 
   This patch alone not, but this patch + the one in my first mail
   (see
   https://www.redhat.com/archives/libvir-list/2008-November/msg00457.html)
   together make it work for me. The first patch fixes the autostart order,
   the second one adds the necessary conn structure.
 
  Oh yes, I totally missed the patch in your first mail. The first patch
  is definitely correct and I'll apply that shortly.
 
 I just added some documentation (same as in virInitialize) to make sure this 
 bug does not get introduced again. New version attached.
 
  While your second 
  patch is also functionally OK, I'm not entirely happy with creating a
  connection object deep inside the QEMU driver code.
 
 Yeah, that was exactly my thought too.
 
  So I'm going to 
  think about whether there's a better way todo that bit.
 
 I looked through the rest of the qemu-initialization and it looks like this 
 is 
 the only point where the conn-object is needed. So a solution would be to 
 directly access the network driver functions. But on the other hand these 
 functions seem all to imply that there is a valid conn available. So we would 
 have to change that, at least for the lookup-by-name case.

I've committed this patch now. I'll send a suggestion for the second patch
shortly...

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


RE: [libvirt] [PATCH] post-release patch for symbols

2008-12-02 Thread Ben Guthro
If I understood the previous conversation correctly, these thread-safe changes 
are necessary for the Java bindings DomainEvent code.

Do we really want those bindings to fall that far behind?


-Original Message-
From: [EMAIL PROTECTED] on behalf of Daniel Veillard
Sent: Tue 12/2/2008 5:30 AM
To: libvir-list@redhat.com
Subject: [libvirt] [PATCH] post-release patch for symbols
 
  Assuming we release a 0.5.1 soon. Maybe the code with threading
should be on a 0.6.0 considering the amount of changes though.

Daniel

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

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


Re: [libvirt] PATCH: 15/28: Reduce return points for storage driver

2008-12-02 Thread Jim Meyering
Daniel P. Berrange [EMAIL PROTECTED] wrote:
 This patch reduces the number of return points in the storage driver
 methods
...
 diff --git a/src/storage_driver.c b/src/storage_driver.c
...
 @@ -893,7 +924,7 @@ storagePoolListVolumes(virStoragePoolPtr

   cleanup:
  for (n = 0 ; n  maxnames ; n++)
 -VIR_FREE(names[i]);
 +VIR_FREE(names[n]);

  memset(names, 0, maxnames);
  return -1;

This might be worth putting in a separate bug-fix patch.
At first I thought this was fixing a serious bug,
but you can see that i is always smaller than maxnames,
so the fix is just plugging a leak.

However, in looking at this I spotted a real problem:
There are numerous statements like this:

memset(names, 0, maxnames);

That zeros out only 1/4 or 1/8 of the memory it should.
It should be doing this:

memset(names, 0, maxnames * sizeof (*names));

These bugs are independent of your 28-part patch, Dan,
i.e., also on the trunk:

  $ git grep memset.names|grep -v sizeof
  src/storage_driver.c:memset(names, 0, nnames);
  src/storage_driver.c:memset(names, 0, nnames);
  src/storage_driver.c:memset(names, 0, maxnames);
  src/storage_driver.c:memset(names, 0, maxnames);
  src/test.c:memset(names, 0, maxnames);
  src/test.c:memset(names, 0, maxnames);

I'll post the fix (relative to the trunk) separately.

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


[libvirt] review fallout: short memset

2008-12-02 Thread Jim Meyering
While reviewing unrelated changes, I spotted a short memset:

char **names;
...
memset(names, 0, maxnames);

That zeros out 1/4 or 1/8 of the memory than it should.
It should be doing this:

memset(names, 0, maxnames * sizeof (*names));

I checked all memset uses and found a total of 6 uses like that.
This fixes them:

From 614b0064eadc3ecef0690d5c65bb531844a3091d Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Tue, 2 Dec 2008 14:49:03 +0100
Subject: [PATCH] fix inadequate initialization in storage and test drivers

* src/storage_driver.c (storageListPools): Set all names entries to 0.
(storageListDefinedPools, storagePoolListVolumes): Likewise.
* src/test.c (testStoragePoolListVolumes): Likewise.
---
 src/storage_driver.c |8 
 src/test.c   |4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/storage_driver.c b/src/storage_driver.c
index 366820b..53388f1 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -347,7 +347,7 @@ storageListPools(virConnectPtr conn,
 free(names[i]);
 names[i] = NULL;
 }
-memset(names, 0, nnames);
+memset(names, 0, nnames * sizeof(*names));
 return -1;
 }

@@ -389,7 +389,7 @@ storageListDefinedPools(virConnectPtr conn,
 free(names[i]);
 names[i] = NULL;
 }
-memset(names, 0, nnames);
+memset(names, 0, nnames * sizeof(*names));
 return -1;
 }

@@ -880,7 +880,7 @@ storagePoolListVolumes(virStoragePoolPtr obj,
 return -1;
 }

-memset(names, 0, maxnames);
+memset(names, 0, maxnames * sizeof(*names));
 for (i = 0 ; i  pool-volumes.count  n  maxnames ; i++) {
 if ((names[n++] = strdup(pool-volumes.objs[i]-name)) == NULL) {
 virStorageReportError(obj-conn, VIR_ERR_NO_MEMORY,
@@ -895,7 +895,7 @@ storagePoolListVolumes(virStoragePoolPtr obj,
 for (n = 0 ; n  maxnames ; n++)
 VIR_FREE(names[i]);

-memset(names, 0, maxnames);
+memset(names, 0, maxnames * sizeof(*names));
 return -1;
 }

diff --git a/src/test.c b/src/test.c
index 7998886..3e942da 100644
--- a/src/test.c
+++ b/src/test.c
@@ -1951,7 +1951,7 @@ testStoragePoolListVolumes(virStoragePoolPtr obj,
 POOL_IS_ACTIVE(privpool, -1);
 int i = 0, n = 0;

-memset(names, 0, maxnames);
+memset(names, 0, maxnames * sizeof(*names));
 for (i = 0 ; i  privpool-volumes.count  n  maxnames ; i++) {
 if ((names[n++] = strdup(privpool-volumes.objs[i]-name)) == NULL) {
 testError(obj-conn, VIR_ERR_NO_MEMORY, %s, _(name));
@@ -1965,7 +1965,7 @@ testStoragePoolListVolumes(virStoragePoolPtr obj,
 for (n = 0 ; n  maxnames ; n++)
 VIR_FREE(names[i]);

-memset(names, 0, maxnames);
+memset(names, 0, maxnames * sizeof(*names));
 return -1;
 }

--
1.6.0.4.1044.g77718

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


Re: [libvirt] PATCH: 15/28: Reduce return points for storage driver

2008-12-02 Thread Daniel P. Berrange
On Tue, Dec 02, 2008 at 03:02:17PM +0100, Jim Meyering wrote:
 Daniel P. Berrange [EMAIL PROTECTED] wrote:
  This patch reduces the number of return points in the storage driver
  methods
 ...
  diff --git a/src/storage_driver.c b/src/storage_driver.c
 ...
  @@ -893,7 +924,7 @@ storagePoolListVolumes(virStoragePoolPtr
 
cleanup:
   for (n = 0 ; n  maxnames ; n++)
  -VIR_FREE(names[i]);
  +VIR_FREE(names[n]);
 
   memset(names, 0, maxnames);
   return -1;
 
 This might be worth putting in a separate bug-fix patch.
 At first I thought this was fixing a serious bug,
 but you can see that i is always smaller than maxnames,
 so the fix is just plugging a leak.
 
 However, in looking at this I spotted a real problem:
 There are numerous statements like this:
 
 memset(names, 0, maxnames);
 
 That zeros out only 1/4 or 1/8 of the memory it should.
 It should be doing this:
 
 memset(names, 0, maxnames * sizeof (*names));

memset() is horribly error prone - also have the classic of getting
the 2  3rd args the wrong way around. How about adding to memory.h
something like

  #define VIR_ZERO(buf, nelement) \
memset(buf, 0, sizeof(*(buf)) * nelement)

also using your xalloc_oversized magic  ?


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] review fallout: short memset

2008-12-02 Thread Daniel P. Berrange
On Tue, Dec 02, 2008 at 03:02:53PM +0100, Jim Meyering wrote:
 While reviewing unrelated changes, I spotted a short memset:
 
 char **names;
 ...
 memset(names, 0, maxnames);
 
 That zeros out 1/4 or 1/8 of the memory than it should.
 It should be doing this:
 
 memset(names, 0, maxnames * sizeof (*names));
 
 I checked all memset uses and found a total of 6 uses like that.
 This fixes them:

ACK to this immediate fix. As per my other mail we should consider
adding a VIR_ZERO() macro for this, to avoid such errors recurring

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


[libvirt] [PATCH] post-release patch for symbols

2008-12-02 Thread Daniel Veillard
  Assuming we release a 0.5.1 soon. Maybe the code with threading
should be on a 0.6.0 considering the amount of changes though.

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/
Index: src/libvirt_sym.version.in
===
RCS file: /data/cvs/libxen/src/libvirt_sym.version.in,v
retrieving revision 1.6
diff -u -r1.6 libvirt_sym.version.in
--- src/libvirt_sym.version.in  21 Nov 2008 12:27:11 -  1.6
+++ src/libvirt_sym.version.in  2 Dec 2008 10:29:08 -
@@ -249,6 +249,8 @@
 
 } LIBVIRT_0.4.5;
 
+LIBVIRT_0.5.1 {
+} LIBVIRT_0.5.0;
 /*  define new API here using predicted next version number  */
 
 
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: 15/28: Reduce return points for storage driver

2008-12-02 Thread Jim Meyering
Daniel P. Berrange [EMAIL PROTECTED] wrote:

 On Tue, Dec 02, 2008 at 03:02:17PM +0100, Jim Meyering wrote:
 Daniel P. Berrange [EMAIL PROTECTED] wrote:
  This patch reduces the number of return points in the storage driver
  methods
 ...
  diff --git a/src/storage_driver.c b/src/storage_driver.c
 ...
  @@ -893,7 +924,7 @@ storagePoolListVolumes(virStoragePoolPtr
 
cleanup:
   for (n = 0 ; n  maxnames ; n++)
  -VIR_FREE(names[i]);
  +VIR_FREE(names[n]);
 
   memset(names, 0, maxnames);
   return -1;

 This might be worth putting in a separate bug-fix patch.
 At first I thought this was fixing a serious bug,
 but you can see that i is always smaller than maxnames,
 so the fix is just plugging a leak.

 However, in looking at this I spotted a real problem:
 There are numerous statements like this:

 memset(names, 0, maxnames);

 That zeros out only 1/4 or 1/8 of the memory it should.
 It should be doing this:

 memset(names, 0, maxnames * sizeof (*names));

 memset() is horribly error prone - also have the classic of getting
 the 2  3rd args the wrong way around. How about adding to memory.h
 something like

   #define VIR_ZERO(buf, nelement) \
 memset(buf, 0, sizeof(*(buf)) * nelement)

Sure, I'll do that after your 28-part change goes in.

But how about a more generic name like MEM_ZERO?

   #define MEM_ZERO(buf, nelement) \
 memset((buf), 0, sizeof(*(buf)) * (nelement))

 also using your xalloc_oversized magic  ?

If you want to automatically handle oversized memset, we'd have
to make it a function and change all users to test the return value.
That'd be pretty invasive.  With malloc/realloc/calloc, where
everyone was already testing the return value, it was easy.
But testing memset's return value isn't useful, so no one does that.

An alternative is to abort on detection of an oversized count*size.
That's not library friendly at all, but better than memory corruption
or other run-time misbehavior.

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


Re: [libvirt] PATCH: 4/28: Thread safety for test driver

2008-12-02 Thread Daniel P. Berrange
On Mon, Dec 01, 2008 at 05:44:14PM +, Daniel P. Berrange wrote:
 On Mon, Dec 01, 2008 at 06:26:03PM +0100, Daniel Veillard wrote:
  On Sun, Nov 30, 2008 at 11:27:14PM +, Daniel P. Berrange wrote:
  I hope it's worth the effort, it's a lot of complexity added.
  One of the things which worries me is that detecting errors will be
  hard, you end up with a locked server that can be far from trivial
  to debug.
  I'm really wondering how we could automate testing or at least ease the
  debug,
 
 It occurred to me that a static analysis tool like CIL really ought to
 be able to check correctness fairly easily. Rich has discussed this a
 little before
 
   http://et.redhat.com/~rjones/cil-analysis-of-libvirt/
 
 Basically since we have a known set of functions which lock the object
 and CIL can give us the code flow within a method, we ought to be able
 to write something that looks at each call of virDomainFindByUUID,
 and then traces the code paths to functiuon return, and validates that
 the unlock call was made. It could likewise check for people calling
 things before acquiring the lock.
 
 Would be an interesting project. :-)

Trying out Rich's demo code I got it to generate before  after control
flow diagrams of the 'domain create' functiuon n the QEMU driver

  http://fedorapeople.org/~berrange/libvirt/qemudDomainCreateBefore.png
  http://fedorapeople.org/~berrange/libvirt/qemudDomainCreateAfter.png

The main problem I had was that CIL doesn't underderstand the 'bool'
data type, so I had to hack  gnulib/lib/c-ctype.{h,c}, src/cgroup.c
and src/xmlrpc.{h,c} to do a s/bool/char/.

Be nice to get rid of these directly in our source tree, or find a compile
flag to automatically turn 'bool' into 'char' ?

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] PATCH: 10/28: Thread safety for LXC driver

2008-12-02 Thread Daniel Veillard
On Sun, Nov 30, 2008 at 11:50:30PM +, Daniel P. Berrange wrote:
 This patch makes the LXC driver thread safe following the same pattern
 as the Test/QEMU drivers

  okay that starts to look familiar,

  cleanup:
  virDomainDefFree(def);
 +if (vm)
 +virDomainObjUnlock(vm);

  Maybe it's just simpler to make virDomainObjUnlock accept and ignore
NULL rather than doing that check in all driver functions.

  Okay, I didn't spot anything, +1

Daniel

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

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


Re: [libvirt] Doubt over Kvm migration support in libvirt 0.5.0

2008-12-02 Thread Chris Lalancette
Anton Protopopov wrote:
 2008/12/2 Shanmuga Rajan [EMAIL PROTECTED]
 mailto:[EMAIL PROTECTED]
 
 I am using libvirt(python) to develop a small application which will
 manage my nodes which running in KVM.
 But when i tried migration i got this error.
 
  vs.migrate(conn, libvirt.VIR_MIGRATE_LIVE,'testtt','192.168.1.82
 http://192.168.1.82',0)
 libvir: error : this function is not supported by the hypervisor:
 virDomainMigrate
 Traceback (most recent call last):
  File stdin, line 1, in ?
  File /usr/lib64/python2.4/site-packages/libvirt.py, line 301, in
 migrate
   if ret is None:raise libvirtError('virDomainMigrate() failed',
 dom=self)
 libvirt.libvirtError: virDomainMigrate() failed this function is not
 supported by the hypervisor: virDomainMigrate
 
 
 i am running kvm-66 taken from Centos-5 Testing repositary and libvirt
 0.5.0( 0.5.0: Nov 25 2008 release note says that support for kvm
 migration included)
 
 See
 https://www.redhat.com/archives/libvir-list/2008-December/msg00027.html

Yes, true, you definitely need updated kvm userland packages for it to work.
However, the error above probably means that you didn't properly update libvirt.
 Either you are still running the older libvirtd binary, or you forgot to
restart libvirtd.  Once you have restarted libvirtd, you'll probably get a
different error message unless you have kvm-79 or later.

-- 
Chris Lalancette

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


Re: [libvirt] PATCH: 15/28: Reduce return points for storage driver

2008-12-02 Thread Daniel P. Berrange
On Tue, Dec 02, 2008 at 03:32:28PM +0100, Jim Meyering wrote:
 Daniel P. Berrange [EMAIL PROTECTED] wrote:
 
  On Tue, Dec 02, 2008 at 03:02:17PM +0100, Jim Meyering wrote:
  Daniel P. Berrange [EMAIL PROTECTED] wrote:
   This patch reduces the number of return points in the storage driver
   methods
  ...
   diff --git a/src/storage_driver.c b/src/storage_driver.c
  ...
   @@ -893,7 +924,7 @@ storagePoolListVolumes(virStoragePoolPtr
  
 cleanup:
for (n = 0 ; n  maxnames ; n++)
   -VIR_FREE(names[i]);
   +VIR_FREE(names[n]);
  
memset(names, 0, maxnames);
return -1;
 
  This might be worth putting in a separate bug-fix patch.
  At first I thought this was fixing a serious bug,
  but you can see that i is always smaller than maxnames,
  so the fix is just plugging a leak.
 
  However, in looking at this I spotted a real problem:
  There are numerous statements like this:
 
  memset(names, 0, maxnames);
 
  That zeros out only 1/4 or 1/8 of the memory it should.
  It should be doing this:
 
  memset(names, 0, maxnames * sizeof (*names));
 
  memset() is horribly error prone - also have the classic of getting
  the 2  3rd args the wrong way around. How about adding to memory.h
  something like
 
#define VIR_ZERO(buf, nelement) \
  memset(buf, 0, sizeof(*(buf)) * nelement)
 
 Sure, I'll do that after your 28-part change goes in.
 
 But how about a more generic name like MEM_ZERO?
 
#define MEM_ZERO(buf, nelement) \
  memset((buf), 0, sizeof(*(buf)) * (nelement))

Sure, works for me.

  also using your xalloc_oversized magic  ?
 
 If you want to automatically handle oversized memset, we'd have
 to make it a function and change all users to test the return value.
 That'd be pretty invasive.  With malloc/realloc/calloc, where
 everyone was already testing the return value, it was easy.
 But testing memset's return value isn't useful, so no one does that.

Ok, lets ignore that bit of the idea - chances are the caller will
already have had the check for oversized data when they allocated
the array they're passing.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] [PATCH 2/2]: Call udevsettle in the appropriate places

2008-12-02 Thread Chris Lalancette
Chris Lalancette wrote:
 Daniel P. Berrange wrote:
 This seems rather overkill when you could just do

 #if defined(UDEVADM) || defined(UDEVSETTLE)
 void virStorageBackendWaitForDevices(virConnectPtr conn)
 {
 #ifdef UDEVADM
 const char *const settleprog[] = { UDEVADM, settle, NULL };
 #else
 const char *const settleprog[] = { UDEVSETTLE, NULL };
 #endif
 int exitstatus;
 
 OK, updated patch attached that basically does the above, and adds the
 configure.in test.

Committed this after talking to DanB about it on IRC.

-- 
Chris Lalancette

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


Re: [libvirt] PATCH: 8/28: Thread safety for domain events code

2008-12-02 Thread Daniel Veillard
On Sun, Nov 30, 2008 at 11:47:21PM +, Daniel P. Berrange wrote:
 This patch adds thread safety to the domain events processing code of
 the QEMU driver. This entailed rather a large refactoring of the domain
 events code and its quite complicated to explain.
 
 - A convenient helper is added for creating event queues
 
 virDomainEventQueuePtr virDomainEventQueueNew(void);
 
 - Convenient helpers are added for creating virDomainEventPtr instances
   from a variety of sources - each driver has different needs
 
virDomainEventPtr virDomainEventNew(int id, const char *name, const 
 unsigned char *uuid, int type, int detail);
virDomainEventPtr virDomainEventNewFromDom(virDomainPtr dom, int type, int 
 detail);
virDomainEventPtr virDomainEventNewFromObj(virDomainObjPtr obj, int type, 
 int detail);
virDomainEventPtr virDomainEventNewFromDef(virDomainDefPtr def, int type, 
 int detail);
 
 - The virDomainEvent struct held a reference to a virDomainPtr object.
   This is replaced with a direct triple (id, name, uuid), avoiding the
   need to instantiate virDomainPtr objects deep inside the QEMU code.
 
 - The virDomainEventQueuePush() method is changed to take a
   virDomainEventPtr object, rather than a virDomainPtr object
 
 These last two changes are important to allow safe re-entrancy of the 
 event dispatch process. The virDomainEventPtr instance can be allocated
 within a critical section lockde on the virDomainObjPtr instance, while
 the event is queued outside the critical section, while only the driver
 lock is held. Without this we'd have to hold the per-driver lock over
 a much larger block of code which hurts concurrancy.
 
 The QEMU driver cannot directly dispatch events, instead we have to
 follow the remote driver and maintain a queue of pending events, and
 use a timer to flush them. Again this is neccessary to improve
 concurrency  provide safe re-entrancy.
 
 Since we have two driver maintaining queues of evnts I add more 
 helper functions to allow code sharing
 
  - Send a single vent to a list of callbacks:
 
   void virDomainEventDispatch(virDomainEventPtr event,
   virDomainEventCallbackListPtr cbs,
   virDomainEventDispatchFunc dispatch,
   void *opaque);
 
  - Send a set of queued events to a list of callbacks
 
   void virDomainEventQueueDispatch(virDomainEventQueuePtr queue,
virDomainEventCallbackListPtr cbs,
virDomainEventDispatchFunc dispatch,
void *opaque);
 
 The virDomainEventDispatchFunc is what is invoked to finally dispatch
 a single event, to a single registered callback. The default impl just
 invokes the callback directly. The QEMU driver, however, uses a wrapper
 which first releases its driver lock, invokes the callback, and then
 re-aquires the lock.
 
 As a further complication it is not safe for virDomainEventUnregister
 to actually remove the callback from the list directly. Instead we
 add a helper that simply marks it as removed, and then actually 
 purge it from a safe point in the code, when its guarenteed that the
 driver isn't in the middle of dispatching.
 
 As well as making the QEMU driver thread safe, this also takes the
 opportunity to refactor the Xen / remote drivers to use more helpers

  I must admit I'm a bit lost ...

 @@ -192,14 +291,18 @@ virDomainEventQueueFree(virDomainEventQu
  virDomainEventQueueFree(virDomainEventQueuePtr queue)
  {
  int i;
 -for ( i=0 ; iqueue-count ; i++ ) {
 -VIR_FREE(queue-events[i]);
 +if (!queue)
 +return;
 +
 +for (i = 0; i  queue-count ; i++) {
 +virDomainEventFree(queue-events[i]);
  }
 +VIR_FREE(queue-events);
  VIR_FREE(queue);
  }

  okay we were leaking queue-events here !

[...]
  struct _virDomainEventCallback {
  virConnectPtr conn;
  virConnectDomainEventCallback cb;
  void *opaque;
  virFreeCallback freecb;
 -
 +int deleted;
  };

  Yup, I'm lost here ... seems we are making something asynchronous here

  struct _virDomainEvent {
 -virDomainPtr dom;
 -int event;
 +int id;
 +char *name;
 +unsigned char uuid[VIR_UUID_BUFLEN];
 +int type;
  int detail;
  };

  Okay we don't want to held references anymore. But the lookups will
require search and locking, I completely fail to build a mental
representation where I can figure out why we don't risk deadlocks there.
There is a risk of lock reentrancy I'm sure but I can't see the model.

[...]
 diff --git a/src/libvirt_sym.version.in b/src/libvirt_sym.version.in
 --- a/src/libvirt_sym.version.in
 +++ b/src/libvirt_sym.version.in
 @@ -382,9 +382,21 @@ [EMAIL PROTECTED]@ {
   virDomainEventCallbackListFree;
   virDomainEventCallbackListRemove;
   virDomainEventCallbackListRemoveConn;
 + virDomainEventCallbackListMarkDelete;
 + 

Re: [libvirt] PATCH: 12/28: Thread safety for UML driver

2008-12-02 Thread Daniel Veillard
On Tue, Dec 02, 2008 at 11:06:38AM +, Daniel P. Berrange wrote:
 On Tue, Dec 02, 2008 at 12:05:10PM +0100, Daniel Veillard wrote:
  On Sun, Nov 30, 2008 at 11:53:17PM +, Daniel P. Berrange wrote:
   This patch makes the UML driver thread safe
  
  [...]
   @@ -1395,21 +1485,27 @@ static int umlListDefinedDomains(virConn
struct uml_driver *driver = conn-privateData;
int got = 0, i;

   +umlDriverLock(driver);
for (i = 0 ; i  driver-domains.count  got  nnames ; i++) {
   +virDomainObjLock(driver-domains.objs[i]);
if (!virDomainIsActive(driver-domains.objs[i])) {
if (!(names[got++] = 
   strdup(driver-domains.objs[i]-def-name))) {
umlReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
 %s, _(failed to allocate space for 
   VM name string));
   +virDomainObjUnlock(driver-domains.objs[i]);
goto cleanup;
}
}
   +virDomainObjUnlock(driver-domains.objs[i]);
}
   +umlDriverUnlock(driver);

return got;

 cleanup:
  
since this is an error code path, I would rather change the label to
  error: ...
 
 That's a good idea - we should define a standard naming for this usage
 
   'cleanup' to be used where error  normal conditions shared the same
 exit path of a method.
 
   'error' to be used where error path is completely separate from the
   normal exit path.

  yup since we're going to have labels on a lot of functions for exit we
should try to standardize their use and name. The set of patches does
this mostly, just a small incremental improvement, and then we can
document it in the HACKING file.

Daniel

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

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


Re: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets

2008-12-02 Thread Daniel P. Berrange
On Tue, Dec 02, 2008 at 11:38:36AM +, Daniel P. Berrange wrote:
 On Mon, Dec 01, 2008 at 12:23:12PM +0100, Gerd v. Egidy wrote:
  Hi Daniel,
  
This patch alone not, but this patch + the one in my first mail
(see
https://www.redhat.com/archives/libvir-list/2008-November/msg00457.html)
together make it work for me. The first patch fixes the autostart order,
the second one adds the necessary conn structure.
  
   Oh yes, I totally missed the patch in your first mail. The first patch
   is definitely correct and I'll apply that shortly.
  
  I just added some documentation (same as in virInitialize) to make sure 
  this 
  bug does not get introduced again. New version attached.
  
   While your second 
   patch is also functionally OK, I'm not entirely happy with creating a
   connection object deep inside the QEMU driver code.
  
  Yeah, that was exactly my thought too.
  
   So I'm going to 
   think about whether there's a better way todo that bit.
  
  I looked through the rest of the qemu-initialization and it looks like this 
  is 
  the only point where the conn-object is needed. So a solution would be to 
  directly access the network driver functions. But on the other hand these 
  functions seem all to imply that there is a valid conn available. So we 
  would 
  have to change that, at least for the lookup-by-name case.
 
 I've committed this patch now. I'll send a suggestion for the second patch
 shortly...

Ok, I think your suggested patch is the best we can manage at this point
in time. Here's an update which also handle it for UML driver, and adds
a check for NULL on connection open.

Daniel

Index: src/qemu_driver.c
===
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.162
diff -u -p -r1.162 qemu_driver.c
--- src/qemu_driver.c   28 Nov 2008 12:03:20 -  1.162
+++ src/qemu_driver.c   2 Dec 2008 11:52:28 -
@@ -138,12 +138,26 @@ static struct qemud_driver *qemu_driver 
 static void
 qemudAutostartConfigs(struct qemud_driver *driver) {
 unsigned int i;
+/* XXX: Figure out a better way todo this. The domain
+ * startup code needs a connection handle in order
+ * to lookup the bridge associated with a virtual
+ * network
+ */
+virConnectPtr conn = virConnectOpen(getuid() ?
+qemu:///session :
+qemu:///system);
+
+if (!conn) {
+qemudLog(QEMUD_ERR, %s,
+ _(Cannot autostart domains, failed to open connection));
+return;
+}
 
 for (i = 0 ; i  driver-domains.count ; i++) {
 virDomainObjPtr vm = driver-domains.objs[i];
 if (vm-autostart 
 !virDomainIsActive(vm)) {
-int ret = qemudStartVMDaemon(NULL, driver, vm, NULL);
+int ret = qemudStartVMDaemon(conn, driver, vm, NULL);
 if (ret  0) {
 virErrorPtr err = virGetLastError();
 qemudLog(QEMUD_ERR, _(Failed to autostart VM '%s': %s\n),
@@ -155,6 +169,8 @@ qemudAutostartConfigs(struct qemud_drive
 }
 }
 }
+
+virConnectClose(conn);
 }
 
 /**
Index: src/uml_driver.c
===
RCS file: /data/cvs/libvirt/src/uml_driver.c,v
retrieving revision 1.6
diff -u -p -r1.6 uml_driver.c
--- src/uml_driver.c2 Dec 2008 11:23:27 -   1.6
+++ src/uml_driver.c2 Dec 2008 11:52:28 -
@@ -117,16 +117,32 @@ static struct uml_driver *uml_driver = N
 static void
 umlAutostartConfigs(struct uml_driver *driver) {
 unsigned int i;
+/* XXX: Figure out a better way todo this. The domain
+ * startup code needs a connection handle in order
+ * to lookup the bridge associated with a virtual
+ * network
+ */
+virConnectPtr conn = virConnectOpen(getuid() ?
+uml:///session :
+uml:///system);
+
+if (!conn) {
+umlLog(UML_ERR, %s,
+   _(Cannot autostart domains, failed to open connection));
+return;
+}
 
 for (i = 0 ; i  driver-domains.count ; i++) {
 if (driver-domains.objs[i]-autostart 
 !virDomainIsActive(driver-domains.objs[i]) 
-umlStartVMDaemon(NULL, driver, driver-domains.objs[i])  0) {
+umlStartVMDaemon(conn, driver, driver-domains.objs[i])  0) {
 virErrorPtr err = virGetLastError();
 umlLog(UML_ERR, _(Failed to autostart VM '%s': %s\n),
  driver-domains.objs[i]-def-name, err-message);
 }
 }
+
+virConnectClose(conn);
 }
 
 

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  

Re: [libvirt] PATCH: 7/28: Thread safety for QEMU driver

2008-12-02 Thread Daniel P. Berrange
On Tue, Dec 02, 2008 at 10:45:32AM +0100, Daniel Veillard wrote:
 On Sun, Nov 30, 2008 at 11:33:06PM +, Daniel P. Berrange wrote:
  This patch makes the QEMU driver threadsafe with the exception of the
  domain events code.
  @@ -229,20 +243,22 @@ qemudStartup(void) {
 [...]
  +error:
  +if (qemu_driver)
  +qemuDriverUnlock(qemu_driver);
   VIR_FREE(base);
  -VIR_FREE(qemu_driver);
  +qemudShutdown();
 
   okay the clean way to free the driver structure
 
  @@ -314,16 +334,16 @@ qemudShutdown(void) {
   if (!qemu_driver)
   return -1;
   
  +qemuDriverLock(qemu_driver);
   virCapabilitiesFree(qemu_driver-caps);
   
   /* shutdown active VMs */
   for (i = 0 ; i  qemu_driver-domains.count ; i++) {
   virDomainObjPtr dom = qemu_driver-domains.objs[i];
  +virDomainObjLock(dom);
   if (virDomainIsActive(dom))
   qemudShutdownVMDaemon(NULL, qemu_driver, dom);
  -if (!dom-persistent)
  -virDomainRemoveInactive(qemu_driver-domains,
  -dom);
  +virDomainObjUnlock(dom);
   }
   
   virDomainObjListFree(qemu_driver-domains);
  @@ -340,6 +360,7 @@ qemudShutdown(void) {
   if (qemu_driver-brctl)
   brShutdown(qemu_driver-brctl);
   
  +qemuDriverUnlock(qemu_driver);
   VIR_FREE(qemu_driver);
   
   return 0;
 
We don't remove anymore the non-persistent domain on shutdown ?
 Is that because we can hook this to the event lifecycle processing ?

Thq qemudShutdown() method is invoked when the daemon itself is
shutting down completely. There's no need to call  the individual
object deletion virDomainRemoveInactive() in this case, because a 
few lines later we do a virDomainObjListFree() which bulk removes
all of them. 

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets

2008-12-02 Thread Gerd v. Egidy
Hi Daniel,

thanks for looking into this.

 +virConnectPtr conn = virConnectOpen(getuid() ?
 +qemu:///session :
 +qemu:///system);
 +
 +if (!conn) {
 +qemudLog(QEMUD_ERR, %s,
 + _(Cannot autostart domains, failed to open
 connection)); +return;
 +}

I thought about that check too, but decided against it, because

a) currently we always call it with conn=NULL, it only fails if you don't use 
a directly bridged network

b) it currently won't do any harm calling with conn=NULL, creating the vm 
fails later on because it can't find the network device.

But if some code relying on the conn-structure is added later on without 
sanity-checks, this check will make sure no bigger harm is done.

Kind regards,

Gerd

-- 
Address (better: trap) for people I really don't want to get mail from:
[EMAIL PROTECTED]

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


Re: [libvirt] [PATCH] Increase initial qemu monitor read timeout

2008-12-02 Thread Daniel P. Berrange
On Mon, Dec 01, 2008 at 05:36:51PM -0500, Cole Robinson wrote:
 See https://bugzilla.redhat.com/show_bug.cgi?id=453491
 
 We've been getting bug reports like the above against virt-manager for a
 while now: installing new VMs consistently throws an error 'Timed out
 while reading monitor startup output'. The user can then just retry the
 install and it will go off without a hitch.
 
 I only just realized that the common thread between the reports is
 physical install media: apparently the qemu monitor isn't too responsive
 while it's waiting for a cold cd drive to spin up.

 The attached patch jacks up the timeout from 3 seconds to 10 seconds
 when doing the initial monitor read. I verified that this fixes the issue.

ACK, seems reasonable, although it'll block the caller for longer, this
is best option with way we currently handle startup. 

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] PATCH: 6/28: Reduce return points in QEMU driver

2008-12-02 Thread Daniel Veillard
On Sun, Nov 30, 2008 at 11:31:56PM +, Daniel P. Berrange wrote:
 This patch reduces the number of return points in the QEMU driver methods
 centralizing all cleanup code. It also removes a bunch of unnecessary
 type casts, since void * automatically casts to any type. Finally it
 separates out variable declarations from variable initializations since
 the initializations will need to be protected with the locked critical
 section.

  IMHO the superfluous cast were harmless so could have been preserved,
but it's not a big deal,

[...]
 +if (vm-state != VIR_DOMAIN_PAUSED) {
 +if (qemudMonitorCommand(driver, vm, stop, info)  0) {
 +qemudReportError(dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
 + %s, _(suspend operation failed));
 +goto cleanup;
 +}
 +vm-state = VIR_DOMAIN_PAUSED;
 +qemudDebug(Reply %s, info);
 +qemudDomainEventDispatch(driver, vm,
 + VIR_DOMAIN_EVENT_SUSPENDED,
 + VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
 +VIR_FREE(info);

  qemudMonitorCommand has no comment on when the reply field may or not
be initialized. The code shows it's only if it returns 0 but that's
worth a comment or VIR_FREE(info); should be moved in the cleanup part.
The code is right but any small change here may generate a leak, and
that's true for most of the entry points.

[...]
  static int qemudDomainSave(virDomainPtr dom,
 const char *path) {

 +cleanup:
 +if (fd != -1)
 +close(fd);
 +VIR_FREE(xml);
 +VIR_FREE(safe_path);
 +VIR_FREE(command);
 +VIR_FREE(info);
 +if (ret != 0)
 +unlink(path);
 +
 +return ret;
  }

Considering the complexity for the function, I would not be surprized
if that patch isn't cleaning up a couple of leaks on errors. Good to
have a systematic freeing.

[...]
 @@ -2271,15 +2346,15 @@ static int qemudDomainRestore(virConnect
def))) {
  qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
   %s, _(failed to assign new VM));
 -virDomainDefFree(def);
 -close(fd);
 -return -1;
 +goto cleanup;
  }
 +def = NULL;

  Hum, okay, but that function is harder to review

[...]
  static int qemudDomainStart(virDomainPtr dom) {
[...]
  ret = qemudStartVMDaemon(dom-conn, driver, vm, NULL);
 -if (ret  0)
 -return ret;
 -qemudDomainEventDispatch(driver, vm,
 - VIR_DOMAIN_EVENT_STARTED,
 - VIR_DOMAIN_EVENT_STARTED_BOOTED);
 -return 0;
 +if (ret != -1)
 +qemudDomainEventDispatch(driver, vm,
 + VIR_DOMAIN_EVENT_STARTED,
 + VIR_DOMAIN_EVENT_STARTED_BOOTED);

  small semantic change, but since -1 is the only error code, fine
[...]
  /* Return the disks name for use in monitor commands */
 -static char *qemudDiskDeviceName(const virDomainPtr dom,
 +static char *qemudDiskDeviceName(const virConnectPtr conn,
   const virDomainDiskDefPtr disk) {

  a bit fo refactoring

[...]
 -static int qemudDomainChangeEjectableMedia(virDomainPtr dom,
 +static int qemudDomainChangeEjectableMedia(virConnectPtr conn,
 +   struct qemud_driver *driver,
 +   virDomainObjPtr vm,
 virDomainDeviceDefPtr dev)

 here too

[...]
 -VIR_FREE(dev);
 +cleanup:
 +virDomainDeviceDefFree(dev);
  return ret;
  }

  hum, that was a leak here apparently

[...]
 +if (asprintf(cmd, pci_del 0 %d, detach-slotnum)  0) {
 +qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
 +cmd = NULL;

  If memory allocation wasn’t  possible,  or  some other error occurs, these
functions will return -1, and the contents of strp is undefined.

  gasp, fine :-)


   Okay, independantly of the threading this sis a good cleanup patch
   which should be applied, +1

Daniel

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

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


Re: [libvirt] [PATCH] Fix a few docs errors

2008-12-02 Thread Daniel P. Berrange
On Mon, Dec 01, 2008 at 05:36:57PM -0500, Cole Robinson wrote:
 The attached patch fixes a few cut and paste docs errors I've stumbled
 upon, and a spelling error in a virsh command output.

ACK.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] PATCH: 9/28: Reduce exit paths for LXC driver

2008-12-02 Thread Daniel Veillard
On Sun, Nov 30, 2008 at 11:49:23PM +, Daniel P. Berrange wrote:
 This patch reduces the number of exit paths in the LXC driver, much like
 the earlier patches for other drivers

  Okay, looks simple, +1 !

Daniel

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

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


Re: [libvirt] libvirt 0.5.0 and KVM migration

2008-12-02 Thread Chris Lalancette
Mickaël Canévet wrote:
 Thanks for your quick answer.
 
 I forgot to restart libvirt on one node.
 
 Now I have an other error message:
 
 libvir: QEMU error : operation failed: failed to start listening VM
 
 Is there any kernel limitation also ? will it work with my distro 2.6.26
 kernel with kvm-79 userspace component or do I have to have kvm-79
 kernel module also, or even kernel then 2.6.26 ?
 
 In other (debian) words, will an apt-get update kvm -t
 experimental (which provides kvm-79) be enough on my lenny (I would
 like to limit unstable or experimental packages) ?

Just an updated kvm-79 userspace *should* work; for instance, in oVirt, we are
running a kvm-79 userspace equivalent on a 2.6.27 kernel, which is slightly
older.  In general, a new kvm userspace should work on an older kernel, but it
is not a heavily tested configuration, so your results might vary.

-- 
Chris Lalancette

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


Re: [libvirt] libvirt 0.5.0 and KVM migration

2008-12-02 Thread Chris Lalancette
Mickaël Canévet wrote:
 On Tue, 2008-12-02 at 09:11 +0100, Chris Lalancette wrote:
 Mickaël Canévet wrote:
 Thanks for your quick answer.

 I forgot to restart libvirt on one node.

 Now I have an other error message:

 libvir: QEMU error : operation failed: failed to start listening VM

 Is there any kernel limitation also ? will it work with my distro 2.6.26
 kernel with kvm-79 userspace component or do I have to have kvm-79
 kernel module also, or even kernel then 2.6.26 ?

 In other (debian) words, will an apt-get update kvm -t
 experimental (which provides kvm-79) be enough on my lenny (I would
 like to limit unstable or experimental packages) ?
 Just an updated kvm-79 userspace *should* work; for instance, in oVirt, we 
 are
 running a kvm-79 userspace equivalent on a 2.6.27 kernel, which is slightly
 older.  In general, a new kvm userspace should work on an older kernel, but 
 it
 is not a heavily tested configuration, so your results might vary.

 OK,
 
 I updated both userspace and kernel module to kvm-79.
 Now migration begins, but:
 - On the source node, the command does not returns and an strace shows
 that it does nothing (wait4(9709, )
 - On the destination node, I can connect threw the VNC display, but
 there seams to have a strange issue with I/O (lot of messages on
 console; sd 0:0:0:0: rejecting I/O to offline device). FYI my VM disk is
 a DRBD over LVM as dual primary.
 - From another PC, I lose a few pings during migration.
 
 Are my I/0 errors comes from my backend storage (DRBD) ?

Probably, it's hard to say without knowing more about your setup (and I don't
really know anything about DRBD, sorry).  In any case, you are probably out of
the realm of libvirt at this point; libvirtd has done the right thing, I think,
so any other errors are probably down to KVM itself.  It might be best to test
the same setup by hand (that is, not use libvirt, but execute the qemu-kvm
binary directly).  If you are still having problems with that, then you can
debug kvm directly and/or mail qemu-devel and kvm-devel.

-- 
Chris Lalancette

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


Re: [libvirt] PATCH: 7/28: Thread safety for QEMU driver

2008-12-02 Thread Daniel Veillard
On Sun, Nov 30, 2008 at 11:33:06PM +, Daniel P. Berrange wrote:
 This patch makes the QEMU driver threadsafe with the exception of the
 domain events code.
 @@ -229,20 +243,22 @@ qemudStartup(void) {
[...]
 +error:
 +if (qemu_driver)
 +qemuDriverUnlock(qemu_driver);
  VIR_FREE(base);
 -VIR_FREE(qemu_driver);
 +qemudShutdown();

  okay the clean way to free the driver structure

 @@ -314,16 +334,16 @@ qemudShutdown(void) {
  if (!qemu_driver)
  return -1;
  
 +qemuDriverLock(qemu_driver);
  virCapabilitiesFree(qemu_driver-caps);
  
  /* shutdown active VMs */
  for (i = 0 ; i  qemu_driver-domains.count ; i++) {
  virDomainObjPtr dom = qemu_driver-domains.objs[i];
 +virDomainObjLock(dom);
  if (virDomainIsActive(dom))
  qemudShutdownVMDaemon(NULL, qemu_driver, dom);
 -if (!dom-persistent)
 -virDomainRemoveInactive(qemu_driver-domains,
 -dom);
 +virDomainObjUnlock(dom);
  }
  
  virDomainObjListFree(qemu_driver-domains);
 @@ -340,6 +360,7 @@ qemudShutdown(void) {
  if (qemu_driver-brctl)
  brShutdown(qemu_driver-brctl);
  
 +qemuDriverUnlock(qemu_driver);
  VIR_FREE(qemu_driver);
  
  return 0;

   We don't remove anymore the non-persistent domain on shutdown ?
Is that because we can hook this to the event lifecycle processing ?

 @@ -1089,18 +1087,38 @@ qemudDispatchVMEvent(int watch, int fd, 
[...]
 +if (failed || quit) {
 +qemudShutdownVMDaemon(NULL, driver, vm);
 +qemudDomainEventDispatch(driver, vm,
 + VIR_DOMAIN_EVENT_STOPPED,
 + quit ?
 + VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN :
 + VIR_DOMAIN_EVENT_STOPPED_FAILED);
 +if (!vm-persistent) {
 +virDomainRemoveInactive(driver-domains,
 +vm);
 +vm = NULL;
 +}
 +}

  seems to be done here now, okay

   I could not spot anything on the locking side, +1

Daniel

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

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


Re: [libvirt] [PATCH] Sanitize qemu monitor output

2008-12-02 Thread Cole Robinson
Daniel P. Berrange wrote:
 On Mon, Dec 01, 2008 at 05:36:34PM -0500, Cole Robinson wrote:
 The attached patch cleans up output we read from the qemu monitor. This
 is a simplified form of a patch I posted awhile ago. From the patch:

 /* The monitor doesn't dump clean output after we have written to
  * it. Every character we write dumps a bunch of useless stuff,
  * so the result looks like cXcoXcomXcommXcommaXcommanXcommand
  * Try to throw away everything before the first full command
  * occurence, and inbetween the command and the newline starting
  * the response
  */


snip

 
 
 This loooks a little overly complex to me, doesn't handle alloction failures
 in strdup correctly  leaks tmpbuf. 

Argh, yeah, I'm always botching this.

 
  if ((commptr = strstr(buf, cmd)))
   memmove(buf, commptr, strlen(commptr)+1);
 

Yes that's much simpler, though it doesn't take into account the control
characters after the command string and before the newline starting the
command response.

We could just do:

 if ((commptr = strstr(buf, cmd)))
  memmove(buf, commptr, strlen(commptr)+1);
 if ((dupptr = strchr(buf, '\n'))
  memmove(buf+strlen(cmd), dupptr, strlen(dupptr)+1);

I was also trying to avoid having the command string in the 'reply', but
it was an unnecessary restriction.

Updated patch attached. It also fixes an error check from the blockstats
monitor command which wouldn't have worked in the first place, but
should be accurate now.

Thanks,
Cole
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 0130d61..0c4226c 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1145,7 +1145,23 @@ qemudMonitorCommand (const struct qemud_driver *driver ATTRIBUTE_UNUSED,
 
 /* Look for QEMU prompt to indicate completion */
 if (buf  ((tmp = strstr(buf, \n(qemu) )) != NULL)) {
-tmp[0] = '\0';
+char *commptr = NULL, *nlptr = NULL;
+
+/* Preserve the newline */
+tmp[1] = '\0';
+
+/* The monitor doesn't dump clean output after we have written to
+ * it. Every character we write dumps a bunch of useless stuff,
+ * so the result looks like cXcoXcomXcommXcommaXcommanXcommand
+ * Try to throw away everything before the first full command
+ * occurence, and inbetween the command and the newline starting
+ * the response
+ */
+if ((commptr = strstr(buf, cmd)))
+memmove(buf, commptr, strlen(commptr)+1);
+if ((nlptr = strchr(buf, '\n')))
+memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1);
+
 break;
 }
 pollagain:
@@ -2596,7 +2612,7 @@ static int qemudDomainChangeEjectableMedia(virDomainPtr dom,
 
 if (qemudMonitorCommand(driver, vm, cmd, reply)  0) {
 qemudReportError(dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- %s, _(cannot change cdrom media));
+ %s, _(could not change cdrom media));
 VIR_FREE(cmd);
 return -1;
 }
@@ -2607,7 +2623,7 @@ static int qemudDomainChangeEjectableMedia(virDomainPtr dom,
 DEBUG (ejectable media change reply: %s, reply);
 if (strstr(reply, \ndevice )) {
 qemudReportError (dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-  %s, _(changing cdrom media failed));
+  _(changing cdrom media failed: %s), reply);
 VIR_FREE(reply);
 VIR_FREE(cmd);
 return -1;
@@ -3143,7 +3159,7 @@ qemudDomainBlockStats (virDomainPtr dom,
  * unlikely to be the name of a block device, we can use this
  * to detect if qemu supports the command.
  */
-if (STRPREFIX (info, info )) {
+if (strstr(info, \ninfo )) {
 qemudReportError (dom-conn, dom, NULL, VIR_ERR_NO_SUPPORT,
   %s,
   _('info blockstats' not supported by this qemu));
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] review fallout: short memset

2008-12-02 Thread Daniel Veillard
On Tue, Dec 02, 2008 at 02:18:43PM +, Daniel P. Berrange wrote:
 On Tue, Dec 02, 2008 at 03:02:53PM +0100, Jim Meyering wrote:
  While reviewing unrelated changes, I spotted a short memset:
  
  char **names;
  ...
  memset(names, 0, maxnames);
  
  That zeros out 1/4 or 1/8 of the memory than it should.
  It should be doing this:
  
  memset(names, 0, maxnames * sizeof (*names));
  
  I checked all memset uses and found a total of 6 uses like that.
  This fixes them:
 
 ACK to this immediate fix. As per my other mail we should consider
 adding a VIR_ZERO() macro for this, to avoid such errors recurring

  +1 to the fix and VIR_ZERO()

Daniel

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

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


Re: [libvirt] [PATCH] post-release patch for symbols

2008-12-02 Thread Daniel Veillard
On Tue, Dec 02, 2008 at 06:58:34AM -0500, Ben Guthro wrote:
 If I understood the previous conversation correctly, these thread-safe 
 changes are necessary for the Java bindings DomainEvent code.
 
 Do we really want those bindings to fall that far behind?

  I agree the java event support is pending the thread safe changes.
When I suggested 0.6.0 that could be as well the next release (or the
one just after).

Daniel

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

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


Re: [libvirt] [PATCH 1/2] Java bindings for domain events

2008-12-02 Thread David Lively
Now that I'm actually wading around in the Java java.nio.channels Select
swamp (getting this implemented), this is looking like it has exactly
the same portability problems as my original code.

This approach requires providing our own Selector (and SelectorProvider,
corresponding handle/timeout Channels, and SelectionKeys), with our own
native implementation of something that can provide select/poll
functionality (because there's no way to get that functionality via Java
for arbitrary unix fds; can't even construct java FileDescriptors from
them in native code because FileDescriptors are declared final).  And we
also must implement Selector.wakeup() that can interrupt a select
operation in another thread (requiring something like the pipe() in my
original code - since the select operation will itself be in native
code, so can't rely on Java thread interruption).

So in the end this looks more like a java.nio.channels.Selector-style
interface wrapped around a libvirt EventImpl (which might as well be the
one copied from qemud).  Certainly that exports a more standard
interface for integration with a foreign event loop, but at this point,
that looks like the only benefit.  For now, I'll continue, assuming the
more standard interface is worth the extra code. The Java side of this
is basically done, with only the JNI code to write.

Just wanted to set expectations ...

Dave


On Wed, 2008-11-19 at 08:34 +0100, Daniel Veillard wrote:
 On Tue, Nov 18, 2008 at 01:12:42PM -0500, David Lively wrote:
  On Tue, 2008-11-18 at 16:51 +, Daniel P. Berrange wrote:
   On Tue, Nov 18, 2008 at 11:06:10AM -0500, David Lively wrote:
The attached patch (against libvirt-java) implements Java bindings for
libvirt domain events.  This version provides a libvirt EventImpl
running in its own Java Thread, and provides per-Connect synchronization
that makes using the bindings thread-safe.  (Note the Domain, Network,
StoragePool, and StorageVol methods also synchronize on their Connect
object, as required by libvirt.  I have similar changes for
NodeDevice.java that need to be made when that code is checked in.)
   
   I don't particularly like the event loop code because it is adding a huge 
   pile of non-portable JNI code that won't work on Windows, which lacks
   pipe() and poll(). Java already provides a portable pure Java API for 
   building a poll() like event loop in form of NIO.
   
 http://www.xhaus.com/alan/python/jynio/select.html
  
  Yeah, Daniel V and I had briefly considered this, and rejected it on the
  basis of it's complicated and (more importantly) some negative
  feedback I hear from our Java folks on the java.nio Select mechanism.
  
  But I agree the java.nio Select mechanism should greatly decrease the
  amount of JNI code in the Java EventImpl.  I need to look over the docs
  again, but I think it's just a matter of implementing a
  SelectableChannel on top of a fd.  (That JNI code will presumably be
  very different in Win32 and Unix, but it should be a relatively small
  amount of JNI code in comparison to my current impl.)
  
  So I'll look over the java.nio Select documentation and start thinking
  about a more portable approach ... (and also talk more with our Java
  folks about their Select gripes).
 
   I guess it's better to invest a bit more time and come up with a
 solution relying as much as possible on Java threading, I/O and
 synchronization. After all we should capitalize as much as possible
 on the portability work done in the Java engine, and limit the
 C part of the bindings to the strict minimum JNI (as much as possible).
   On one hand we want the bindings to be the easiest possible to use
 and avoid threading limitation imposed to the client code, on the other
 hand limit the C part on those issue, of course that means growing
 the java side of the bindings, but it really should be easier to
 maintain and port than equivalent C code, even if NIO is not the
 nicest Java API :-\
 
 Daniel
 


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


[libvirt] [PATCH] bridge: set MTU of tap iface to the same value of the bridge

2008-12-02 Thread Eduardo Habkost
Hi,

This is a follow-up to the RFC patch sent by Chris Wright, at:
https://www.redhat.com/archives/libvir-list/2008-November/msg00225.html

With this patch, instead of hardcoding the MTU to an high value, we set
the MTU of the tap interface to the same MTU value set on the bridge
interface. The current code uses the default tap MTU value and lowers
the bridge interface MTU without any need, killing performance.

I will send another patch later for allowing the tap MTU to be configured
on the XML, but the behavior added by this patch will be kept as the
default. The default behavior should be enough for most use cases where
a larger MTU is wanted for the bridged interfaces.

Signed-off-by: Eduardo Habkost [EMAIL PROTECTED]
---
 src/bridge.c |   98 ++
 1 files changed, 98 insertions(+), 0 deletions(-)

diff --git a/src/bridge.c b/src/bridge.c
index 4090163..60334b6 100644
--- a/src/bridge.c
+++ b/src/bridge.c
@@ -276,6 +276,98 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED,
 #endif
 
 /**
+ * ifGetMtu
+ * @ctl: bridge control pointer
+ * @ifname: interface name get MTU for
+ *
+ * This function gets the @mtu value set for a given interface @ifname.
+ *
+ * Returns the MTU value in case of success.
+ * On error, returns -1 and sets errno accordingly
+ */
+static int ifGetMtu(brControl *ctl, const char *ifname)
+{
+struct ifreq ifr;
+int len;
+
+if (!ctl || !ifname) {
+errno = EINVAL;
+return -1;
+}
+
+if ((len = strlen(ifname)) =  BR_IFNAME_MAXLEN) {
+errno = EINVAL;
+return -1;
+}
+
+memset(ifr, 0, sizeof(struct ifreq));
+
+strncpy(ifr.ifr_name, ifname, len);
+ifr.ifr_name[len] = '\0';
+
+if (ioctl(ctl-fd, SIOCGIFMTU, ifr))
+return -1;
+
+return ifr.ifr_mtu;
+
+}
+
+/**
+ * ifSetMtu:
+ * @ctl: bridge control pointer
+ * @ifname: interface name to set MTU for
+ * @mtu: MTU value
+ *
+ * This function sets the @mtu for a given interface @ifname.  Typically
+ * used on a tap device to set up for Jumbo Frames.
+ *
+ * Returns 0 in case of success or an errno code in case of failure.
+ */
+static int ifSetMtu(brControl *ctl, const char *ifname, int mtu)
+{
+struct ifreq ifr;
+int len;
+
+if (!ctl || !ifname)
+return EINVAL;
+
+if ((len = strlen(ifname)) =  BR_IFNAME_MAXLEN)
+return EINVAL;
+
+memset(ifr, 0, sizeof(struct ifreq));
+
+strncpy(ifr.ifr_name, ifname, len);
+ifr.ifr_name[len] = '\0';
+ifr.ifr_mtu = mtu;
+
+return ioctl(ctl-fd, SIOCSIFMTU, ifr) == 0 ? 0 : errno;
+}
+
+/**
+ * brSetInterfaceMtu
+ * @ctl: bridge control pointer
+ * @bridge: name of the bridge interface
+ * @ifname: name of the interface whose MTU we want to set
+ *
+ * Sets the interface mtu to the same MTU of the bridge
+ *
+ * Returns 0 in case of success or an errno code in case of failure.
+ */
+static int brSetInterfaceMtu(brControl *ctl,
+ const char *bridge,
+ const char *ifname)
+{
+int mtu = ifGetMtu(ctl, bridge);
+
+fprintf(stderr, mtu: %d\n, mtu);
+
+if (mtu  0)
+return errno;
+
+return ifSetMtu(ctl, ifname, mtu);
+}
+
+/**
  * brAddTap:
  * @ctl: bridge control pointer
  * @bridge: the bridge name
@@ -334,6 +426,12 @@ brAddTap(brControl *ctl,
 }
 
 if (ioctl(fd, TUNSETIFF, try) == 0) {
+/* We need to set the interface MTU before adding it
+ * to the bridge, because the bridge will have its
+ * MTU adjusted automatically when we add the new interface.
+ */
+if ((errno = brSetInterfaceMtu(ctl, bridge, try.ifr_name)))
+goto error;
 if ((errno = brAddInterface(ctl, bridge, try.ifr_name)))
 goto error;
 if ((errno = brSetInterfaceUp(ctl, try.ifr_name, 1)))
-- 
1.5.5.GIT

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


[libvirt] Allowing interface type=none for network interface definition

2008-12-02 Thread Gihan Munasinghe
Hi 


I am writing a management stack for our platform using libvirt. We have many 
DomU images which all have PV drivers loaded for network.I want to start these 
DomU guests with only the PV network device being visible from within the DomU.

This is a requirement in the platform and hopefully, I presume there are other 
people with the same requirement..

But when creating a server, libvirt sends (type ioemu) tag to xend. Which make all of our vm's to 
have 2 network cards. Using the normal xm command I can send type=none ( vif = [ 
mac=00:16:3e:00:a5:57,bridge=eth0,script=vif-bridge,type=none]) to xend. Which will 
send qeum-dm -net none switch which solves the problem.

After doing some investigation, I did some code change in libvirt0.5.0 to support a attribute called none with in interface type=, that will send xend the tag (type none). Allowing qeum-dm to be configured with -net none switch.  ;-) 

So you can define your network tag as 


interface type='none' !-- This will send qeum-dm -net none switch --
   !-- Other configurations will work as normal --
 mac address='00:16:3e:00:a5:57'/
 source bridge='eth0'/
 target dev='vif47.0'/
/interface

I this the proper way of doing it?  :-\ 

Would this be a general functionality that libvirt community will be interested in (I think this add bit more flexibility to libvirt of the way user want to write their own management stack). If so could I contribute the patch to the mailing list. 

Looking forward to hearing from you all, I am new to libvirt therefore any comments will be greatly appreciated  :-)   

Thanks 
Gihan  


--
Gihan Munasinghe
RD Team Leader
XCalibre Communications Ltd.
www.flexiscale.com

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


Re: [libvirt] Allowing interface type=none for network interface definition

2008-12-02 Thread Daniel P. Berrange
On Tue, Dec 02, 2008 at 09:24:24PM +, Gihan Munasinghe wrote:
 Hi 
 
 I am writing a management stack for our platform using libvirt. We have 
 many DomU images which all have PV drivers loaded for network.I want to 
 start these DomU guests with only the PV network device being visible from 
 within the DomU.
 
 This is a requirement in the platform and hopefully, I presume there are 
 other people with the same requirement..
 
 But when creating a server, libvirt sends (type ioemu) tag to xend. Which 
 make all of our vm's to have 2 network cards. Using the normal xm command I 
 can send type=none ( vif = [ 
 mac=00:16:3e:00:a5:57,bridge=eth0,script=vif-bridge,type=none]) to xend. 
 Which will send qeum-dm -net none switch which solves the problem.
 
 After doing some investigation, I did some code change in libvirt0.5.0 to 
 support a attribute called none with in interface type=, that will 
 send xend the tag (type none). Allowing qeum-dm to be configured with -net 
 none switch.  ;-) 
 So you can define your network tag as 
 
 interface type='none' !-- This will send qeum-dm -net none switch --
!-- Other configurations will work as normal --
  mac address='00:16:3e:00:a5:57'/
  source bridge='eth0'/
  target dev='vif47.0'/
 /interface
 
 I this the proper way of doing it?  :-\ 

The type attribute is used to specify how the network card is connected
to the host networking. You're after something that changes what is 
exposed tto the guest.  The XML format for network interfaces already has 
support for a model element which accomplishes this. eg, for KVM we can
select VirtIO network card with:

interface type='bridge'
model type=virtio/
source bridge='br0'/
mac address='00:16:3e:00:a5:57'/
/interface

This 'model' is not currently used in the Xen driver for libvirt, so
we should implement it. A value of 'none' doesn't really make sense.
For Xen = 3.1.0, libvirt will no longer append type=ioemu by default.
This lets XenD configure by rtl8139 nic, and PV driver back/front.

So I think we just need to allow an explicit choice to override this

A couple of choices, either follow XenD's naming

   model type=ioemu/
   model type=netback/

Or take into account we might want to allow for full range of QEMU
nic choices, even if XenD doesn't (yet) expose it

   model type=rtl8139/
   model type=e1000/
   model type=ne2k_pci/
   model type=netback/

I think i tend towards the latter, since its more consistent with naming
in QEMU driver.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


[libvirt] how to install updated python bindings

2008-12-02 Thread ente linux
hi

i was trying to install libvirt 0.5 on my centos 5 machine which by default
have libvirt 0.4. But after installing from the source of libvirt, still the
output of i get from python remains that of 0.4
import libvirt
 libvirt.getVersion()
4006

how could i install the new version... Do i need to give  any option while
configuring or making.

thanks and regards
visco
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Re: [PATCH] bridge: set MTU of tap iface to the same value of the bridge

2008-12-02 Thread Chris Wright
* Eduardo Habkost ([EMAIL PROTECTED]) wrote:
 +static int brSetInterfaceMtu(brControl *ctl,
 + const char *bridge,
 + const char *ifname)
 +{
 +int mtu = ifGetMtu(ctl, bridge);
 +
 +fprintf(stderr, mtu: %d\n, mtu);

Is that just a bit of leftover debugging?

Other than that, looks good to me.

ACK

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


[libvirt] is libvirt-python default in libvirt compilation from source?

2008-12-02 Thread Shanmuga Rajan
I was trying to install libvirt 0.5.0 from source..
It seems it went well. but when i tried to import libvirt module in python
i got error no module named libvirt

then i searched entire system for libvirt.py then i came to know that
python bindings for libvirt is not installed.

is it that python bindings for libvirt not default in compilation?

if i have to install python bindings then what option should i use?

i tried to find option from ./configure --help from libvirt source directory
but i didnt get anything.

Can any one help me in this.

Thanks and Regards,
Shan

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


RE: [libvirt] broken pipe?

2008-12-02 Thread Itamar Heim
As no one answered my previous problem., I'm trying to build latest
libvirt to be able to be more productive and find out if happens on later
versions.

Some silly questions to save time (on FC10 64):

1.   Can one build without xen-devel (the autobuild.sh does not care
for --without-xen
configure: error: You must install the Xen development package to compile
Xen driver with -lxenstore
(I installed xen-devel to bypass for now)

2.   checking libxml2 xml2-config = 2.5.0 ... configure: error: Could
not find libxml2 anywhere (see config.log for details)
but
Package libxml2-2.7.2-2.fc10.x86_64 already installed and latest version?
(checked at latest master and 0.50 tag)

3.   already found the SDL patch is part of 0.50. since the build
failed, I installed the rpm:

rpm -i /tmp/libvirt-0.5.0-1.fc9.x86_64.rpm

warning: /tmp/libvirt-0.5.0-1.fc9.x86_64.rpm: Header V3 DSA signature:
NOKEY, key ID de95bc1f

error: Failed dependencies:

 libgnutls.so.13()(64bit) is needed by libvirt-0.5.0-1.fc9.x86_64

 libgnutls.so.13(GNUTLS_1_3)(64bit) is needed by
libvirt-0.5.0-1.fc9.x86_64

 

I checked with 0.4.6 as well, I assume this is a difference between
fc9/fc10 somehow (since fc10 comes with 0.4.6)

4.   as for the python-libvirt rpm

Seems the libvirt-python spec does not check for some dependency of cygwin
(why would I need that on fedora)?

Traceback (most recent call last):

  File ./test_libvirt.py, line 2, in module

import libvirt

  File /usr/lib64/python2.5/site-packages/libvirt.py, line 12, in
module

import cygvirtmod as libvirtmod

ImportError: No module named cygvirtmod

 

Thanks, 

  Itamar

 

 

Thanks,

   Itamar

 

 

From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Itamar Heim
Sent: Tuesday, December 02, 2008 1:08 AM
To: Libvir-list@redhat.com
Subject: [libvirt] broken pipe?

 

Hi,

 

I have a problem with a small python script (using fedora 10 with its
shipped rpm's - libvirt-0.4.6-3.fc10.x86_64,
libvirt-python-0.4.6-3.fc10.x86_64)

I'm running a simple stupid libvirt python script.

I happen to want to use SDL, and it fails on Could not initialize SDL -
exiting.

While I need to solve this error (i.e., find out in which version this
patch was included to allow me to specify the DISPLAY
http://www.mail-archive.com/libvir-list@redhat.com/msg08764.html)

 

I am more troubled by the fact that running the script again gives a
socket/pipe error until I restart libvirtd:

 

Run #1:

 

./test_libvirt.py

libvir: QEMU error : internal error QEMU quit during console startup

Could not initialize SDL - exiting

Traceback (most recent call last):

  File ./test_libvirt.py, line 37, in module

vm = c.createLinux(x, 0)

  File /usr/lib64/python2.5/site-packages/libvirt.py, line 892, in
createLinux

if ret is None:raise libvirtError('virDomainCreateLinux() failed',
conn=self)

libvirt.libvirtError: internal error QEMU quit during console startup

 

Run #2:

./test_libvirt.py

libvir: Remote error : socket closed unexpectedly

Traceback (most recent call last):

  File ./test_libvirt.py, line 37, in module

vm = c.createLinux(x, 0) # what a lousy function name

  File /usr/lib64/python2.5/site-packages/libvirt.py, line 892, in
createLinux

if ret is None:raise libvirtError('virDomainCreateLinux() failed',
conn=self)

libvirt.libvirtError: socket closed unexpectedly

libvir: Remote error : Broken pipe

 

Code:

#!/usr/bin/env python

import libvirt

c = libvirt.open('qemu:///system')

x = domain type='kvm'

  nameyossi2/name

  memory512000/memory  currentMemory512000/currentMemory

  vcpu1/vcpu

  ostype arch='i686' machine='pc'hvm/typeboot dev='hd'/
/os

  featuresacpi/  /features

  clock offset='utc'/

  devices

emulator/usr/bin/qemu-kvm/emulator

disk type='file' device='disk'

  source file='/mnt/sda2/images/winxp-acpi.vmdk'/

  target dev='hda' bus='ide'/

/disk

input type='tablet' bus='usb'/input type='mouse' bus='ps2'/

graphics type='sdl'/

  /devices

/domain

vm = c.createLinux(x, 0)

 

Thanks,

   Itamar

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