date:20100118

Re: [libvirt] Current cpu and memory usage for Host and Domains

2010-01-18 Thread Daniel Veillard

On Sun, Jan 17, 2010 at 01:02:33PM +, Daniel P. Berrange wrote:
 On Fri, Jan 15, 2010 at 11:52:09AM -0800, su disheng wrote:
  Seem there is no way to get host CPU usage in libvirt API? I can get
  freeMemory by  virNodeGetFreeMemory, but no cpu usage API.
  How about add a new api for it? The host CPU usage is also important for VM
  creation and migration decision, like the free memory.
  Yes, I know, I can got it from top...:)
 
 We've historically thought that monitoring of the host OS is out of
 scope of the libvirt API, since there are already a huge variety of
 monitoring services / APIs available for people.  I wonder if we need
 to reconsider that now we have the VMWare drivers though - apps using
 libvirt don't get to installl stuff in VMWare host OS, so the only
 access to some of this host CPU data is probably via the VMWare APIs.
 Which in turn suggests we'd need to expose some minimal hosting API
 for this in libvirt

  I think in both case it boils down to be able to rely on a single
access method for the monitoring of virtualization and host capacity,
the pressure has increased with usage or remote access and new
hypervisors.
  It's not like providing those new APIs would be hard, but we still
have pure virtualization code to focuse on,

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] remove useless comparisons in lxc_driver, vbox_tmpl.c

2010-01-18 Thread Jim Meyering

Here are two change sets.
They remove useless comparisons of the style

-if (def-nets[i]-mac) {

where the mac member is declared as an array.

From d22b04d2684d5a16f797d8df0c2ef036b5b1b2ce Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 18 Jan 2010 09:49:57 +0100
Subject: [PATCH 1/2] lxc_driver: remove useless comparison

* src/lxc/lxc_driver.c (lxcSetupInterfaces): Remove always-true
array-is-non-NULL test.  git grep 'mac\[.*\];'|grep -F .h
src/conf/domain_conf.h:unsigned char mac[VIR_MAC_BUFLEN];
---
 src/lxc/lxc_driver.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 86606c7..94ec874 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -834,7 +834,7 @@ static int lxcSetupInterfaces(virConnectPtr conn,
 goto error_exit;
 }

-if (def-nets[i]-mac) {
+{
 char macaddr[VIR_MAC_STRING_BUFLEN];
 virFormatMacAddr(def-nets[i]-mac, macaddr);
 if (0 != (rc = setMacAddr(containerVeth, macaddr))) {
--
1.6.6.638.g2bc54


From c751c0cf3352b08d674bb59efd987f106fc1fffb Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 18 Jan 2010 09:58:57 +0100
Subject: [PATCH 2/2] vbox_tmpl.c: remove useless array-is-non-NULL comparisons

* src/vbox/vbox_tmpl.c (vboxStorageVolDelete): Remove always-true
array-is-non-NULL test.  git grep 'key\[.*\];'|grep -F .h
src/datatypes.h:char key[PATH_MAX];
(vboxStorageVolGetInfo): Likewise.
(vboxStorageVolGetXMLDesc): Likewise.
(vboxStorageVolGetPath): Likewise.
(vboxDomainDefineXML): Likewise. (but now with mac[])
---
 src/vbox/vbox_tmpl.c |   16 ++--
 1 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 07696c0..e40c848 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -4084,9 +4084,7 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr 
conn, const char *xml) {
 }

 VBOX_UTF8_TO_UTF16(macaddrvbox, MACAddress);
-if (def-nets[i]-mac) {
-adapter-vtbl-SetMACAddress(adapter, MACAddress);
-}
+adapter-vtbl-SetMACAddress(adapter, MACAddress);
 VBOX_UTF16_FREE(MACAddress);
 }
 }
@@ -6645,9 +6643,6 @@ static int vboxStorageVolDelete(virStorageVolPtr vol,
 int i = 0;
 int j = 0;

-if (!vol-key)
-return ret;
-
 vboxUtf8toIID(vol-conn, vol-key, hddIID);
 if (!hddIID)
 return ret;
@@ -6774,8 +6769,7 @@ static int vboxStorageVolGetInfo(virStorageVolPtr vol, 
virStorageVolInfoPtr info
 vboxIID   *hddIID= NULL;
 nsresult rc;

-if (   !vol-key
-|| !info)
+if (!info)
 return ret;

 vboxUtf8toIID(vol-conn, vol-key, hddIID);
@@ -6824,9 +6818,6 @@ static char *vboxStorageVolGetXMLDesc(virStorageVolPtr 
vol, unsigned int flags A
 int defOk = 0;
 nsresult rc;

-if (!vol-key)
-return ret;
-
 memset(pool, 0, sizeof(pool));
 memset(def, 0, sizeof(def));

@@ -6919,9 +6910,6 @@ static char *vboxStorageVolGetPath(virStorageVolPtr vol) {
 vboxIID   *hddIID= NULL;
 nsresult rc;

-if (!vol-key)
-return ret;
-
 vboxUtf8toIID(vol-conn, vol-key, hddIID);
 if (!hddIID)
 return ret;
--
1.6.6.638.g2bc54

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

[libvirt] [PATCH] storage_backend_fs.c: do not ignore probe failure

2010-01-18 Thread Jim Meyering

When virStorageBackendProbeTarget fails, it returns -1 or -2.
The two uses below obviously intended to handle those cases
differently, but due to a wrong comparison, they always treated a
real (e.g., open) failure (-1) just like an ignorable wrong file type
failure (-2).

FYI, coverity reported that the goto cleanup statement was
unreachable, which was true in each case, but hardly the real problem.

From c9ba79daf124afbab351741192bbaf7110a199dd Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 18 Jan 2010 10:34:53 +0100
Subject: [PATCH] storage_backend_fs.c: do not ignore probe failure

* src/storage/storage_backend_fs.c (virStorageBackendFileSystemRefresh):
Correct two error-handling bugs.  The documented intent is to ignore
non-regular files, yet due to a comparison  0 (rather than != 0)
all errors were handled that way.

(vboxStorageVolDelete): Likewise.
(vboxDomainDefineXML): Likewise.
---
 src/storage/storage_backend_fs.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 4fe40b3..fa72f9c 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1,7 +1,7 @@
 /*
  * storage_backend_fs.c: storage backend for FS and directory handling
  *
- * Copyright (C) 2007-2009 Red Hat, Inc.
+ * Copyright (C) 2007-2010 Red Hat, Inc.
  * Copyright (C) 2007-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -562,7 +562,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn,
 backingStore,
 vol-allocation,
 vol-capacity,
-vol-target.encryption)  0)) 
{
+vol-target.encryption) != 
0)) {
 if (ret == -1)
 goto cleanup;
 else {
@@ -603,7 +603,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn,
 if ((ret = virStorageBackendProbeTarget(conn,
 vol-backingStore,
 NULL, NULL, NULL,
-NULL))  0) {
+NULL)) != 0) {
 if (ret == -1)
 goto cleanup;
 else {
--
1.6.6.638.g2bc54

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

[libvirt] [PATCH] secret_driver.c: remove dead cleanup code

2010-01-18 Thread Jim Meyering

Here's another example of appropriate use of assert.
The while loop removed below is currently guaranteed never
to be executed, since list is always NULL at that point.
However, you can argue that simply removing the loop is a little
risky: what if someone changes things to goto cleanup before
list reaches the final NULL?  That's why I added the assertion:
to catch the potential (albeit unlikely) future coding error.

It would be a shame to include real run-time code to detect
a situation like that that will probably never arise.  And if you
do, technically you'd have to document that the function would
return some sort of error code if a developer changes its guts
in a way that violates this invariant.  That would be nonsensical.

From 6baa2b0b8f9963fa60ff0946afcefd57586e2720 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 18 Jan 2010 10:46:57 +0100
Subject: [PATCH] secret_driver.c: remove dead cleanup code

* src/secret/secret_driver.c (loadSecrets): Remove dead code
after cleanup:.  At that point, list is known to be NULL.
Instead, add an assertion.
Include assert.h.
---
 src/secret/secret_driver.c |   10 +++---
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 1eef468..b4b4f8a 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -1,7 +1,7 @@
 /*
  * secret_driver.c: local driver for secret manipulation API
  *
- * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2009-2010 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -28,6 +28,7 @@
 #include string.h
 #include sys/stat.h
 #include unistd.h
+#include assert.h

 #include internal.h
 #include base64.h
@@ -518,12 +519,7 @@ loadSecrets(virConnectPtr conn, virSecretDriverStatePtr 
driver,
 ret = 0;

 cleanup:
-while (list != NULL) {
-virSecretEntryPtr s;
-
-s = listUnlink(list);
-secretFree(s);
-}
+assert (list == NULL);
 if (dir != NULL)
 closedir(dir);
 return ret;
--
1.6.6.638.g2bc54

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

Re: [libvirt] [PATCH v2] Implement CPU topology support for QEMU driver

2010-01-18 Thread Jiri Denemark

On Sun, Jan 17, 2010 at 12:57:12 +, Daniel P. Berrange wrote:
  @@ -2112,8 +2148,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
   ADD_ARG_LIT(-mem-path);
   ADD_ARG_LIT(driver-hugepage_path);
   }
  +
  +if (!(smp = qemudBuildCommandLineSmp(conn, def, qemuCmdFlags)))
  +goto error;
  +
   ADD_ARG_LIT(-smp);
  -ADD_ARG_LIT(vcpus);
  +ADD_ARG_LIT(smp);
  +VIR_FREE(smp);
 
 
 If you've got an allocated string, then just use 'ADD_ARG(smp)' and which
 avoids the strdup() that ADD_ARG_LIT does and avoids need for VIR_FREE
 too. Also you should move the qemudBuildCommandLineSmp() call to *after*
 the ADD_ARG_LIT(-smp) line, otherwise you can leak 'smp' on OOM handling
 in the ADD_ARG_LIT(-smp) call.
 
 
 ACK, if you make that minor memory handling fix before committing

Right. I don't think I have commit rights so I can either send a v3 or someone
else can change it when committing this patch...

Jirka

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

Re: [libvirt] [PATCH] secret_driver.c: remove dead cleanup code

2010-01-18 Thread Daniel P. Berrange

On Mon, Jan 18, 2010 at 10:53:31AM +0100, Jim Meyering wrote:
 Here's another example of appropriate use of assert.
 The while loop removed below is currently guaranteed never
 to be executed, since list is always NULL at that point.
 However, you can argue that simply removing the loop is a little
 risky: what if someone changes things to goto cleanup before
 list reaches the final NULL?  That's why I added the assertion:
 to catch the potential (albeit unlikely) future coding error.

I don't really consider that a net win. If we leave the current code in
there and someone adds a 'goto cleanup' in future  enhancement, then 
everything will work as design. If we replace the current code with an
assert, then it will abort(), or silently do nothing  thus leak if 
compiled with -DNDEBUG.

So while this is technically dead code at this time I don't think we 
should be removing it from the cleanup: block. The cleanup: blocks 
should be pessimistic about considering what has been cleaned up already,
even if this results in possible dead code warnings. We've had many 
actual bugs from cleanup: blocks not free'ing stuff they should have 
done, many of those introduced after refactorings of original code. 

I realize this means that some automated code checkers like Coverity
will always complain about certain things, but these are not actual
bugs. 

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] remove useless comparisons in lxc_driver, vbox_tmpl.c

2010-01-18 Thread Daniel P. Berrange

On Mon, Jan 18, 2010 at 10:20:18AM +0100, Jim Meyering wrote:
 Here are two change sets.
 They remove useless comparisons of the style
 
 -if (def-nets[i]-mac) {
 
 where the mac member is declared as an array.
 
 From d22b04d2684d5a16f797d8df0c2ef036b5b1b2ce Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 18 Jan 2010 09:49:57 +0100
 Subject: [PATCH 1/2] lxc_driver: remove useless comparison
 
 * src/lxc/lxc_driver.c (lxcSetupInterfaces): Remove always-true
 array-is-non-NULL test.  git grep 'mac\[.*\];'|grep -F .h
 src/conf/domain_conf.h:unsigned char mac[VIR_MAC_BUFLEN];
 ---
  src/lxc/lxc_driver.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
 index 86606c7..94ec874 100644
 --- a/src/lxc/lxc_driver.c
 +++ b/src/lxc/lxc_driver.c
 @@ -834,7 +834,7 @@ static int lxcSetupInterfaces(virConnectPtr conn,
  goto error_exit;
  }
 
 -if (def-nets[i]-mac) {
 +{
  char macaddr[VIR_MAC_STRING_BUFLEN];
  virFormatMacAddr(def-nets[i]-mac, macaddr);
  if (0 != (rc = setMacAddr(containerVeth, macaddr))) {
 --
 1.6.6.638.g2bc54
 
 
 From c751c0cf3352b08d674bb59efd987f106fc1fffb Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 18 Jan 2010 09:58:57 +0100
 Subject: [PATCH 2/2] vbox_tmpl.c: remove useless array-is-non-NULL comparisons
 
 * src/vbox/vbox_tmpl.c (vboxStorageVolDelete): Remove always-true
 array-is-non-NULL test.  git grep 'key\[.*\];'|grep -F .h
 src/datatypes.h:char key[PATH_MAX];
 (vboxStorageVolGetInfo): Likewise.
 (vboxStorageVolGetXMLDesc): Likewise.
 (vboxStorageVolGetPath): Likewise.
 (vboxDomainDefineXML): Likewise. (but now with mac[])
 ---
  src/vbox/vbox_tmpl.c |   16 ++--
  1 files changed, 2 insertions(+), 14 deletions(-)
 
 diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
 index 07696c0..e40c848 100644
 --- a/src/vbox/vbox_tmpl.c
 +++ b/src/vbox/vbox_tmpl.c
 @@ -4084,9 +4084,7 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr 
 conn, const char *xml) {
  }
 
  VBOX_UTF8_TO_UTF16(macaddrvbox, MACAddress);
 -if (def-nets[i]-mac) {
 -adapter-vtbl-SetMACAddress(adapter, MACAddress);
 -}
 +adapter-vtbl-SetMACAddress(adapter, MACAddress);
  VBOX_UTF16_FREE(MACAddress);
  }
  }
 @@ -6645,9 +6643,6 @@ static int vboxStorageVolDelete(virStorageVolPtr vol,
  int i = 0;
  int j = 0;
 
 -if (!vol-key)
 -return ret;
 -
  vboxUtf8toIID(vol-conn, vol-key, hddIID);
  if (!hddIID)
  return ret;
 @@ -6774,8 +6769,7 @@ static int vboxStorageVolGetInfo(virStorageVolPtr vol, 
 virStorageVolInfoPtr info
  vboxIID   *hddIID= NULL;
  nsresult rc;
 
 -if (   !vol-key
 -|| !info)
 +if (!info)
  return ret;
 
  vboxUtf8toIID(vol-conn, vol-key, hddIID);
 @@ -6824,9 +6818,6 @@ static char *vboxStorageVolGetXMLDesc(virStorageVolPtr 
 vol, unsigned int flags A
  int defOk = 0;
  nsresult rc;
 
 -if (!vol-key)
 -return ret;
 -
  memset(pool, 0, sizeof(pool));
  memset(def, 0, sizeof(def));
 
 @@ -6919,9 +6910,6 @@ static char *vboxStorageVolGetPath(virStorageVolPtr 
 vol) {
  vboxIID   *hddIID= NULL;
  nsresult rc;
 
 -if (!vol-key)
 -return ret;
 -
  vboxUtf8toIID(vol-conn, vol-key, hddIID);
  if (!hddIID)
  return ret;
 --

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] storage_backend_fs.c: do not ignore probe failure

2010-01-18 Thread Daniel P. Berrange

On Mon, Jan 18, 2010 at 10:40:26AM +0100, Jim Meyering wrote:
 When virStorageBackendProbeTarget fails, it returns -1 or -2.
 The two uses below obviously intended to handle those cases
 differently, but due to a wrong comparison, they always treated a
 real (e.g., open) failure (-1) just like an ignorable wrong file type
 failure (-2).
 
 FYI, coverity reported that the goto cleanup statement was
 unreachable, which was true in each case, but hardly the real problem.

 @@ -562,7 +562,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn,
  backingStore,
  vol-allocation,
  vol-capacity,
 -vol-target.encryption)  
 0)) {
 +vol-target.encryption) != 
 0)) {
  if (ret == -1)
  goto cleanup;
  else {
 @@ -603,7 +603,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn,
  if ((ret = virStorageBackendProbeTarget(conn,
  vol-backingStore,
  NULL, NULL, NULL,
 -NULL))  0) {
 +NULL)) != 0) {
  if (ret == -1)
  goto cleanup;
  else {

I don't understand how this is making any difference. The method
virStorageBackendProbeTarget can return 0, -1 or -2, so

virStorageBackendProbeTarget  0  catches the -1 and -2 cases
virStorageBackendProbeTarget != 0 catches the -1 and -2 cases

I only see this change making a difference if one of the error return values
is a positive integer greater than 0, but we don't have that here. What am
I missing ? 

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 14/34] Fixed char device stuff

2010-01-18 Thread Daniel P. Berrange

On Fri, Jan 15, 2010 at 04:17:44PM +0100, Daniel Veillard wrote:
 On Fri, Jan 08, 2010 at 05:23:10PM +, Daniel P. Berrange wrote:
  Temp hack
  ---
   src/qemu/qemu_monitor_text.c |9 +++--
   1 files changed, 3 insertions(+), 6 deletions(-)
  
  diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
  index 52cd97c..2b8c1e8 100644
  --- a/src/qemu/qemu_monitor_text.c
  +++ b/src/qemu/qemu_monitor_text.c
  @@ -1678,14 +1678,12 @@ cleanup:
   int qemuMonitorTextGetPtyPaths(qemuMonitorPtr mon,
  virHashTablePtr paths)
   {
  -const char *cmd = info chardev;
   char *reply = NULL;
   int ret = -1;
   
  -if (qemuMonitorCommand(mon, cmd, reply)  0) {
  -qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
  - _(failed to retrieve chardev info in qemu with 
  '%s'),
  - cmd);
  +if (qemuMonitorCommand(mon, info chardev, reply)  0) {
  +qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED, %s,
  + _(failed to retrieve chardev info in qemu with 
  'info chardev'));
   goto cleanup;
   }
   
  @@ -1747,7 +1745,6 @@ int qemuMonitorTextGetPtyPaths(qemuMonitorPtr mon,
   ret = 0;
   
   cleanup:
  -VIR_FREE(cmd);
   VIR_FREE(reply);
   return ret;
   }
 
   Looks strictly equivalent, ACK,

Sorry, this commit message should not have been left like this. There is
one subtle difference. The original code is free()ing a 'constant'
string. This is pretty much a guarenteed SEGV in malloc code.

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] storage_backend_fs.c: do not ignore probe failure

2010-01-18 Thread Jim Meyering

Daniel P. Berrange wrote:
 On Mon, Jan 18, 2010 at 10:40:26AM +0100, Jim Meyering wrote:
 When virStorageBackendProbeTarget fails, it returns -1 or -2.
 The two uses below obviously intended to handle those cases
 differently, but due to a wrong comparison, they always treated a
 real (e.g., open) failure (-1) just like an ignorable wrong file type
 failure (-2).

 FYI, coverity reported that the goto cleanup statement was
 unreachable, which was true in each case, but hardly the real problem.
...

 I don't understand how this is making any difference. The method
 virStorageBackendProbeTarget can return 0, -1 or -2, so

 virStorageBackendProbeTarget  0  catches the -1 and -2 cases
 virStorageBackendProbeTarget != 0 catches the -1 and -2 cases

 I only see this change making a difference if one of the error return values
 is a positive integer greater than 0, but we don't have that here. What am
 I missing ?

Good catch.
I changed the wrong thing.  Real problem: misplaced ).
Here's the correct patch:

From 8834728541e38c6843c6941457596bba18ca4ae3 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 18 Jan 2010 10:34:53 +0100
Subject: [PATCH] storage_backend_fs.c: do not ignore probe failure

* src/storage/storage_backend_fs.c (virStorageBackendFileSystemRefresh):
Correct parentheses.  The documented intent is to ignore non-regular
files, yet due to a parenthesization error all errors were handled
that way.
---
 src/storage/storage_backend_fs.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 4fe40b3..b03e4e9 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1,7 +1,7 @@
 /*
  * storage_backend_fs.c: storage backend for FS and directory handling
  *
- * Copyright (C) 2007-2009 Red Hat, Inc.
+ * Copyright (C) 2007-2010 Red Hat, Inc.
  * Copyright (C) 2007-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -562,7 +562,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn,
 backingStore,
 vol-allocation,
 vol-capacity,
-vol-target.encryption)  0)) 
{
+vol-target.encryption))  0) 
{
 if (ret == -1)
 goto cleanup;
 else {
--
1.6.6.638.g2bc54

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

Re: [libvirt] [PATCH] storage_backend_fs.c: do not ignore probe failure

2010-01-18 Thread Daniel P. Berrange

On Mon, Jan 18, 2010 at 01:37:34PM +0100, Jim Meyering wrote:
 Daniel P. Berrange wrote:
  On Mon, Jan 18, 2010 at 10:40:26AM +0100, Jim Meyering wrote:
  When virStorageBackendProbeTarget fails, it returns -1 or -2.
  The two uses below obviously intended to handle those cases
  differently, but due to a wrong comparison, they always treated a
  real (e.g., open) failure (-1) just like an ignorable wrong file type
  failure (-2).
 
  FYI, coverity reported that the goto cleanup statement was
  unreachable, which was true in each case, but hardly the real problem.
 ...
 
  I don't understand how this is making any difference. The method
  virStorageBackendProbeTarget can return 0, -1 or -2, so
 
  virStorageBackendProbeTarget  0  catches the -1 and -2 cases
  virStorageBackendProbeTarget != 0 catches the -1 and -2 cases
 
  I only see this change making a difference if one of the error return values
  is a positive integer greater than 0, but we don't have that here. What am
  I missing ?
 
 Good catch.
 I changed the wrong thing.  Real problem: misplaced ).
 Here's the correct patch:
 
 From 8834728541e38c6843c6941457596bba18ca4ae3 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 18 Jan 2010 10:34:53 +0100
 Subject: [PATCH] storage_backend_fs.c: do not ignore probe failure
 
 * src/storage/storage_backend_fs.c (virStorageBackendFileSystemRefresh):
 Correct parentheses.  The documented intent is to ignore non-regular
 files, yet due to a parenthesization error all errors were handled
 that way.
 ---
  src/storage/storage_backend_fs.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/storage/storage_backend_fs.c 
 b/src/storage/storage_backend_fs.c
 index 4fe40b3..b03e4e9 100644
 --- a/src/storage/storage_backend_fs.c
 +++ b/src/storage/storage_backend_fs.c
 @@ -1,7 +1,7 @@
  /*
   * storage_backend_fs.c: storage backend for FS and directory handling
   *
 - * Copyright (C) 2007-2009 Red Hat, Inc.
 + * Copyright (C) 2007-2010 Red Hat, Inc.
   * Copyright (C) 2007-2008 Daniel P. Berrange
   *
   * This library is free software; you can redistribute it and/or
 @@ -562,7 +562,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn,
  backingStore,
  vol-allocation,
  vol-capacity,
 -vol-target.encryption)  
 0)) {
 +vol-target.encryption))  
 0) {
  if (ret == -1)
  goto cleanup;
  else {

ACK, makes sense now :-)

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] Bugfix for RHEL 5.4+ ?? -- Re: Synchronous commands block virsh?

2010-01-18 Thread Andreas Kurz

Hi list,

I found this thread from December:

http://www.mail-archive.com/libvir-list@redhat.com/msg18595.htm

 that starting with version 0.7.4 libvirtd will no longer block  other API 
calls, which would be extremely useful in HA clusters in combination with live 
migration. 

Will this fix find its way into RHEL 5?

Regards,
Andreas

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

Re: [libvirt] [PATCH 15/34] Auto-add disk controllers based on defined disks

2010-01-18 Thread Daniel P. Berrange

On Fri, Jan 15, 2010 at 04:24:16PM +0100, Daniel Veillard wrote:
 On Fri, Jan 08, 2010 at 05:23:11PM +, Daniel P. Berrange wrote:
  Existing applications using libvirt are not aware of the disk
  controller concept. Thus, after parsing the disk definitions
  in the XML, it is neccessary to create controller elements
  to satisfy all requested disks, as per their defined drive
  addresses
  
  * src/conf/domain_conf.c, src/conf/domain_conf.h,
src/libvirt_private.syms: Add virDomainDefAddDiskControllers()
method for populating disk controllers, and call it after
parsing disk definitions.
  * src/qemu/qemu_conf.c: Call virDomainDefAddDiskControllers()
when doing ARGV - XML conversion
  * tests/qemuxml2argvdata/qemuxml2argv*.xml: Add disk controller
data to all data files which don't have it already
 
  +/*
  + * Based on the declared address type=drive info for any disks,
  + * add neccessary drive controllers which are not already present
  + * in the XML. This is for compat with existing apps which will
  + * not know/care about controller info in the XML
  + */
  +int virDomainDefAddDiskControllers(virDomainDefPtr def)
  +{
  +if (virDomainDefAddDiskControllersForType(def,
  +  
  VIR_DOMAIN_CONTROLLER_TYPE_SCSI,
  +  VIR_DOMAIN_DISK_BUS_SCSI)  
  0)
  +return -1;
  +
  +if (virDomainDefAddDiskControllersForType(def,
  +  
  VIR_DOMAIN_CONTROLLER_TYPE_FDC,
  +  VIR_DOMAIN_DISK_BUS_FDC)  0)
  +return -1;
  +
  +if (virDomainDefAddDiskControllersForType(def,
  +  
  VIR_DOMAIN_CONTROLLER_TYPE_IDE,
  +  VIR_DOMAIN_DISK_BUS_IDE)  0)
  +return -1;
  +
  +return 0;
  +}
 
 IIRC a previous patch in the serie disk controler are not sorted,
 so the order here is just arbitrary, right ?

The code is not sorting controller types, merely controller indexes
within a given type. So SCSI / FDC / IDE can appear in any order,
but if you add an extra controller of type 'SCSI', it will be inserted
in correct position wrt existing SCSI controllers.

 
 
  --- a/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.xml
  +++ b/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.xml
  @@ -21,5 +21,7 @@
 readonly/
 address type='drive' controller='0' bus='0' unit='0'/
   /disk
  +controller type='ide' index='0'/
  +controller type='fdc' index='0'/
 /devices
   /domain
 
   strange here in the output we get ide before fdc, I would have assume
 the reverse based on the new function. I hope the order in generated
 output won't change randomly.
 
   Except that remark looks fine, 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 18/34] Add device info to serial, parallel, channel, input fs devices

2010-01-18 Thread Daniel P. Berrange

On Fri, Jan 15, 2010 at 05:26:33PM +0100, Daniel Veillard wrote:
 On Fri, Jan 08, 2010 at 05:23:14PM +, Daniel P. Berrange wrote:
  Although the serial, parallel, chanel, input  fs devices do
  not have PCI address info, they can all have device aliases.
  Thus it neccessary to associate the virDomainDeviceInfo data
  with them all.
  
  * src/conf/domain_conf.c, src/conf/domain_conf.h: Add hooks for
parsing / formatting device info for serial, parallel, channel
input and fs devices.
  * docs/schemas/domain.rng: Associate device info with character
devices, input  fs device
  ---
   docs/schemas/domain.rng |   12 
   src/conf/domain_conf.c  |   67 
  ++
   src/conf/domain_conf.h  |4 +++
   3 files changed, 71 insertions(+), 12 deletions(-)
  
  diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
  index a32ce45..f4bef7b 100644
  --- a/docs/schemas/domain.rng
  +++ b/docs/schemas/domain.rng
  @@ -600,6 +600,9 @@
 /interleave
   /group
 /choice
  +  optional
  +   ref name=address/
  +  /optional
   /element
 /define
 define name=filesystemtgt
  @@ -990,6 +993,9 @@
 /optional
   /element
 /optional
  +  optional
  +   ref name=address/
  +  /optional
   /interleave
 /define
 define name=qemucdevSrcType
  @@ -1119,6 +1125,9 @@
 interleave
   ref name=qemucdevSrcDef/
   ref name=guestfwdTarget/
  +   optional
  + ref name=address/
  +   /optional
 /interleave
   /element
 /define
  @@ -1139,6 +1148,9 @@
 /choice
   /attribute
 /optional
  +  optional
  +   ref name=address/
  +  /optional
   /element
 /define
 define name=hostdev
 
   Hum, unless I'm mistaken this also allow to add a PCI like address
 on input even if it would be ignored, maybe this should be tightened to
 just allow the info

That is correct - it'll allow any of the device info at the parsing
stage. Some hypervisors may support PCI based serial, parallel, devices
and input devices. Ideally, we should make all the hypervisor drivers
check for  reject devices with addressing schemes they don't support
but that's alot of work.

We might need to thing about exposing a way for hypervisor drivers to
register fine grained parser features. So a HV driver could indicate
that it only supports devices X, Y  Z with addressing schemes A  B.

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] logging: confirm that we want to ignore a write error

2010-01-18 Thread Jim Meyering

Ignoring a write failure is a big deal, so when you do that deliberately,
it's worth marking in such a way that code analyzers don't report a
false positive.  Until a year or so ago, people thought using (void)
would be enough.  But newer gcc warns in spite of that (for functions
marked with the warn_unused_result attribute), so now we use the
ignore_value function from gnulib.

A coverity warning prompted the change below, but if we declare
saferead with the warn_unused_result attribute, gcc would, too.

From 62d2c7f69608407afa9a4bf38f0d29ea3eec88f5 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 18 Jan 2010 11:51:01 +0100
Subject: [PATCH] logging: confirm that we want to ignore a write error

* src/util/logging.c (virLogMessage): Include ignore-value.h.
Use it to ignore the return value of safewrite.
Use STDERR_FILENO, rather than 2.
* bootstrap (modules): Add ignore-value.
* gnulib: Update to latest, for ignore-value that is now LGPLv2+.
---
 .gnulib|2 +-
 bootstrap  |1 +
 src/util/logging.c |5 +++--
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/.gnulib b/.gnulib
index 4c52807..146d914 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 4c52807f41f238cf0e352317b2dc54f9ba0f0c4f
+Subproject commit 146d9145073e62a2096a2d6b33f75e93908fedf3
diff --git a/bootstrap b/bootstrap
index c07d851..aec5d05 100755
--- a/bootstrap
+++ b/bootstrap
@@ -76,6 +76,7 @@ getpass
 gettext
 gitlog-to-changelog
 gnumakefile
+ignore-value
 inet_pton
 ioctl
 maintainer-makefile
diff --git a/src/util/logging.c b/src/util/logging.c
index 6bd8469..3b3c309 100644
--- a/src/util/logging.c
+++ b/src/util/logging.c
@@ -1,7 +1,7 @@
 /*
  * logging.c: internal logging and debugging
  *
- * Copyright (C) 2008 Red Hat, Inc.
+ * Copyright (C) 2008, 2010 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -34,6 +34,7 @@
 #include syslog.h
 #endif

+#include ignore-value.h
 #include logging.h
 #include memory.h
 #include util.h
@@ -579,7 +580,7 @@ void virLogMessage(const char *category, int priority, 
const char *funcname,
msg, len, virLogOutputs[i].data);
 }
 if ((virLogNbOutputs == 0)  (flags != 1))
-safewrite(2, msg, len);
+ignore_value (safewrite(STDERR_FILENO, msg, len));
 virLogUnlock();

 VIR_FREE(msg);
--
1.6.6.638.g2bc54

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

[libvirt] Release 0.4.1 of libvirt-java

2010-01-18 Thread Bryan Kearney

I have just released 0.4.1 of libvirt java. There are 2 main items in 
this release:


- Better null checking in for Scheduled Parameters which should fix the 
issues reported on the list.
- Error Callbacks to provide better handling of errors encountered by 
libvirt (virConnSetErrorFunc and virSetErrorFunc).


You can access the latest version via the following means:

Source Code: http://www.libvirt.org/git/?p=libvirt-java.git;a=summary
Bundled Source (tarball and SRPM): http://libvirt.org/sources/java/
Maven: http://libvirt.org/maven2/
RPMS are making their way through F-11 and F12 build systems

Thank you!

-- bk

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

[libvirt] 0.7.5 errors in configure script

2010-01-18 Thread Avi Weit


Hi,

I am using Ubuntu 9.10 and downloaded and extracted libvirt.0.7.5. I have
issued:
./configure --prefix=$HOME/usr --without-xen

but I get: You must install device-mapper-devel/libdevmapper = 1.0.0 to
compile libvirt

Below is a snippet of the final configure output:

checking for showmount... no
checking for pvcreate... no
checking for vgcreate... no
checking for lvcreate... no
checking for pvremove... no
checking for vgremove... no
checking for lvremove... no
checking for vgchange... no
checking for vgscan... no
checking for pvs... no
checking for vgs... no
checking for lvs... no
checking for iscsiadm... no
checking for DEVMAPPER... no
checking libdevmapper.h usability... no
checking libdevmapper.h presence... no
checking for libdevmapper.h... no
checking for dm_task_run in -ldevmapper... no
configure: error: You must install device-mapper-devel/libdevmapper =
1.0.0 to compile libvirt

/dev/mapper exists on the system.

Any help is appreciated.

Thanks,
- Avi

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

Re: [libvirt] 0.7.5 errors in configure script

2010-01-18 Thread Daniel P. Berrange

On Mon, Jan 18, 2010 at 05:04:26PM +0200, Avi Weit wrote:
 
 Hi,
 
 I am using Ubuntu 9.10 and downloaded and extracted libvirt.0.7.5. I have
 issued:
 ./configure --prefix=$HOME/usr --without-xen
 
 but I get: You must install device-mapper-devel/libdevmapper = 1.0.0 to
 compile libvirt
 
 Below is a snippet of the final configure output:
 
 checking for showmount... no
 checking for pvcreate... no
 checking for vgcreate... no
 checking for lvcreate... no
 checking for pvremove... no
 checking for vgremove... no
 checking for lvremove... no
 checking for vgchange... no
 checking for vgscan... no
 checking for pvs... no
 checking for vgs... no
 checking for lvs... no
 checking for iscsiadm... no
 checking for DEVMAPPER... no
 checking libdevmapper.h usability... no
 checking libdevmapper.h presence... no
 checking for libdevmapper.h... no
 checking for dm_task_run in -ldevmapper... no
 configure: error: You must install device-mapper-devel/libdevmapper =
 1.0.0 to compile libvirt
 
 /dev/mapper exists on the system.

That is a device file created by the kernel/udev. 

What libvirt wants is the  device mapper library, ie libdevmapper.so
typically provided by  a device-mapper-devel/libdevmapper package, or
equivalent for your distro

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] 0.7.5 errors in configure script

2010-01-18 Thread Frédéric Grelot


 checking for dm_task_run in -ldevmapper... no
 configure: error: You must install device-mapper-devel/libdevmapper
 =
 1.0.0 to compile libvirt
 
 /dev/mapper exists on the system.
 

You need the *-devel version of those packages (they provide include files in 
particular).
I think it is something like libdevmapper-devel.
Note that the dev stands for device, while the devel stands for 
development files.

You may get few other errors like this one, don't forget to install *-devel 
versions of the packages that they complain about.


Frederic.

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

Re: [libvirt] 0.7.5 errors in configure script

2010-01-18 Thread Matthias Bolte

2010/1/18 Frédéric Grelot fredericg...@yahoo.fr:

 checking for dm_task_run in -ldevmapper... no
 configure: error: You must install device-mapper-devel/libdevmapper
 =
 1.0.0 to compile libvirt

 /dev/mapper exists on the system.


 You need the *-devel version of those packages (they provide include files in 
 particular).
 I think it is something like libdevmapper-devel.
 Note that the dev stands for device, while the devel stands for 
 development files.

 You may get few other errors like this one, don't forget to install *-devel 
 versions of the packages that they complain about.


 Frederic.


On Fedora development packages end in -devel. On Debian/Ubuntu
development packages end in -dev. The necessary package for Ubuntu is
called:

  libdevmapper-dev

Matthias

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

[libvirt] [PATCH v3] Implement CPU topology support for QEMU driver

2010-01-18 Thread Jiri Denemark

QEMU's command line equivalent for the following domain XML fragment
vcpus2/vcpus
cpu ...
...
topology sockets='1' cores='2', threads='1'/
/cpu

is

-smp 2,sockets=1,cores=2,threads=1

This syntax was introduced in QEMU-0.12.

Version 2 changes:
- -smp argument build split into a separate function
- always add ,sockets=S,cores=C,threads=T to -smp if qemu supports it
- use qemuParseCommandLineKeywords for command line parsing

Version 3 changes:
- ADD_ARG_LIT = ADD_ARG and line reordering in qemudBuildCommandLine
- rebased

Signed-off-by: Jiri Denemark jdene...@redhat.com
---
 src/qemu/qemu_conf.c |  157 -
 src/qemu/qemu_conf.h |3 +-
 2 files changed, 143 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 024b2ba..1fbb86a 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1,7 +1,7 @@
 /*
  * qemu_conf.c: QEMU configuration management
  *
- * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2008, 2009, 2010 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -1119,6 +1119,10 @@ static unsigned int qemudComputeCmdFlags(const char 
*help,
 flags |= QEMUD_CMD_FLAG_DEVICE;
 if (strstr(help, -sdl))
 flags |= QEMUD_CMD_FLAG_SDL;
+if (strstr(help, cores=) 
+strstr(help, threads=) 
+strstr(help, sockets=))
+flags |= QEMUD_CMD_FLAG_SMP_TOPOLOGY;
 
 if (version = 9000)
 flags |= QEMUD_CMD_FLAG_VNC_COLON;
@@ -2676,6 +2680,39 @@ no_memory:
 goto cleanup;
 }
 
+static char *
+qemudBuildCommandLineSmp(virConnectPtr conn,
+ const virDomainDefPtr def,
+ int qemuCmdFlags)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferVSprintf(buf, %lu, def-vcpus);
+
+if ((qemuCmdFlags  QEMUD_CMD_FLAG_SMP_TOPOLOGY)) {
+/* sockets, cores, and threads are either all zero
+ * or all non-zero, thus checking one of them is enough */
+if (def-cpu  def-cpu-sockets) {
+virBufferVSprintf(buf, ,sockets=%u, def-cpu-sockets);
+virBufferVSprintf(buf, ,cores=%u, def-cpu-cores);
+virBufferVSprintf(buf, ,threads=%u, def-cpu-threads);
+}
+else {
+virBufferVSprintf(buf, ,sockets=%lu, def-vcpus);
+virBufferVSprintf(buf, ,cores=%u, 1);
+virBufferVSprintf(buf, ,threads=%u, 1);
+}
+}
+
+if (virBufferError(buf)) {
+virBufferFreeAndReset(buf);
+virReportOOMError(conn);
+return NULL;
+}
+
+return virBufferContentAndReset(buf);
+}
+
 
 /*
  * Constructs a argv suitable for launching qemu with config defined
@@ -2694,7 +2731,6 @@ int qemudBuildCommandLine(virConnectPtr conn,
   const char *migrateFrom) {
 int i;
 char memory[50];
-char vcpus[50];
 char boot[VIR_DOMAIN_BOOT_LAST];
 struct utsname ut;
 int disableKQEMU = 0;
@@ -2708,6 +2744,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
 char uuid[VIR_UUID_STRING_BUFLEN];
 char domid[50];
 char *cpu;
+char *smp;
 
 uname_normalize(ut);
 
@@ -2850,7 +2887,6 @@ int qemudBuildCommandLine(virConnectPtr conn,
  * is not supported, then they're out of luck anyway
  */
 snprintf(memory, sizeof(memory), %lu, def-maxmem/1024);
-snprintf(vcpus, sizeof(vcpus), %lu, def-vcpus);
 snprintf(domid, sizeof(domid), %d, def-id);
 
 ADD_ENV_LIT(LC_ALL=C);
@@ -2913,8 +2949,11 @@ int qemudBuildCommandLine(virConnectPtr conn,
 ADD_ARG_LIT(-mem-path);
 ADD_ARG_LIT(driver-hugepage_path);
 }
+
 ADD_ARG_LIT(-smp);
-ADD_ARG_LIT(vcpus);
+if (!(smp = qemudBuildCommandLineSmp(conn, def, qemuCmdFlags)))
+goto error;
+ADD_ARG(smp);
 
 if (qemuCmdFlags  QEMUD_CMD_FLAG_NAME) {
 ADD_ARG_LIT(-name);
@@ -4647,6 +4686,27 @@ error:
 }
 
 
+static virCPUDefPtr
+qemuInitGuestCPU(virConnectPtr conn,
+ virDomainDefPtr dom)
+{
+if (!dom-cpu) {
+virCPUDefPtr cpu;
+
+if (VIR_ALLOC(cpu)  0) {
+virReportOOMError(conn);
+return NULL;
+}
+
+cpu-type = VIR_CPU_TYPE_GUEST;
+cpu-match = VIR_CPU_MATCH_EXACT;
+dom-cpu = cpu;
+}
+
+return dom-cpu;
+}
+
+
 static int
 qemuParseCommandLineCPU(virConnectPtr conn,
 virDomainDefPtr dom,
@@ -4656,10 +4716,8 @@ qemuParseCommandLineCPU(virConnectPtr conn,
 const char *p = val;
 const char *next;
 
-if (VIR_ALLOC(cpu)  0)
-goto no_memory;
-
-cpu-type = VIR_CPU_TYPE_GUEST;
+if (!(cpu = qemuInitGuestCPU(conn, dom)))
+goto error;
 
 do {
 if (*p == '\0' || *p == ',')
@@ -4703,7 +4761,6 @@ qemuParseCommandLineCPU(virConnectPtr conn,
 }
 } while ((p = next));
 
-dom-cpu =

Re: [libvirt] 0.7.5 errors in configure script

2010-01-18 Thread Avi Weit

Matthias Bolte matthias.bo...@googlemail.com wrote on 18/01/2010
17:52:15:

 Matthias Bolte matthias.bo...@googlemail.com
 18/01/2010 17:52

 To

 Frédéric Grelot fredericg...@yahoo.fr

 cc

 Avi Weit/Haifa/i...@ibmil, libvir-list@redhat.com

 Subject

 Re: [libvirt] 0.7.5 errors in configure script

 2010/1/18 Frédéric Grelot fredericg...@yahoo.fr:
 
  checking for dm_task_run in -ldevmapper... no
  configure: error: You must install device-mapper-devel/libdevmapper
  =
  1.0.0 to compile libvirt
 
  /dev/mapper exists on the system.
 
 
  You need the *-devel version of those packages (they provide
 include files in particular).
  I think it is something like libdevmapper-devel.
  Note that the dev stands for device, while the devel stands
 for development files.
 
  You may get few other errors like this one, don't forget to
 install *-devel versions of the packages that they complain about.
 
 
  Frederic.
 

 On Fedora development packages end in -devel. On Debian/Ubuntu
 development packages end in -dev. The necessary package for Ubuntu is
 called:

   libdevmapper-dev

 Matthias

Thanks Matthias, Frédéric! After installing package libdevmapper-dev for my
Ubuntu box - configure competed successfully. Trying now to compile and
install.. (make; make install).

- Avi


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

Re: [libvirt] [PATCH] qemu_driver: don't leak a virDomainDeviceDef buffer

2010-01-18 Thread Daniel P. Berrange

On Mon, Jan 18, 2010 at 04:58:37PM +0100, Jim Meyering wrote:
 At first I was going to call virDomainDeviceDefFree only if (dev),
 but saw that it handles a NULL dev just fine, so it's better to skip
 the test altogether, just as we do for many other free-like functions.
 
 From ea8511d709492f5cdc152a1eaccbccd05f036648 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 18 Jan 2010 16:55:36 +0100
 Subject: [PATCH] qemu_driver: don't leak a virDomainDeviceDef buffer
 
 * src/qemu/qemu_driver.c (qemudDomainAttachDevice): Don't leak dev.
 ---
  src/qemu/qemu_driver.c |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 365921f..1aa8af6 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -6062,11 +6062,11 @@ cleanup:
  if (cgroup)
  virCgroupFree(cgroup);
 
 -if (ret  0  dev != NULL) {
 +if (ret  0  dev != NULL)
  if (qemuDomainSetDeviceOwnership(dom-conn, driver, dev, 1)  0)
  VIR_WARN0(Fail to restore disk device ownership);
 -virDomainDeviceDefFree(dev);
 -}
 +virDomainDeviceDefFree(dev);
 +
  if (vm)
  virDomainObjUnlock(vm);
  qemuDriverUnlock(driver);
 --

Your fix is reasonable, but can you leave it out of GIT, because there are
a huge number of other related problems in this bit code which I've got 
patches almost ready for.

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] qemu_driver: don't leak a virDomainDeviceDef buffer

2010-01-18 Thread Jim Meyering

Daniel P. Berrange wrote:
...
 Your fix is reasonable, but can you leave it out of GIT, because there are
 a huge number of other related problems in this bit code which I've got
 patches almost ready for.

Sure.  I'll defer it.

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

[libvirt] XML for the boot=on option

2010-01-18 Thread Shi Jin

Hi, there,

In order to install windows 2008 using the virtio driver, I had to modify the 
KVM command to include the boot=on option like:
 -drive file=path/virtioWin08.raw,if=virtio,index=0,boot=on

However, I haven't been able to find a corresponding libvirt XML element that 
can enable this boot=on option. Is there a way to do this?

Thanks a lot.
--
Shi Jin, PhD


  

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

[libvirt] [PATCH] xen_driver: don't leak a parsed-config buffer

2010-01-18 Thread Jim Meyering

Another leak:

From 10a428a2ed1ddbe4da2b4fbd7690da5a0fe0420b Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 18 Jan 2010 18:02:17 +0100
Subject: [PATCH] xen_driver: don't leak a parsed-config buffer

* src/xen/xen_driver.c (xenUnifiedDomainXMLFromNative): Also
free conf before returning.
---
 src/xen/xen_driver.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 4911c9e..72f56ae 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1,7 +1,7 @@
 /*
  * xen_driver.c: Unified Xen driver.
  *
- * Copyright (C) 2007, 2008, 2009 Red Hat, Inc.
+ * Copyright (C) 2007-2010 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
@@ -1199,6 +1199,8 @@ xenUnifiedDomainXMLFromNative(virConnectPtr conn,

 cleanup:
 virDomainDefFree(def);
+if (conf)
+virConfFree(conf);
 return ret;
 }

--
1.6.6.638.g2bc54

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

Re: [libvirt] [PATCH] qemu_driver: don't leak a virDomainDeviceDef buffer

2010-01-18 Thread Matthias Bolte

2010/1/18 Jim Meyering j...@meyering.net:
 At first I was going to call virDomainDeviceDefFree only if (dev),
 but saw that it handles a NULL dev just fine, so it's better to skip
 the test altogether, just as we do for many other free-like functions.

 From ea8511d709492f5cdc152a1eaccbccd05f036648 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 18 Jan 2010 16:55:36 +0100
 Subject: [PATCH] qemu_driver: don't leak a virDomainDeviceDef buffer

 * src/qemu/qemu_driver.c (qemudDomainAttachDevice): Don't leak dev.
 ---
  src/qemu/qemu_driver.c |    6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 365921f..1aa8af6 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -6062,11 +6062,11 @@ cleanup:
     if (cgroup)
         virCgroupFree(cgroup);

 -    if (ret  0  dev != NULL) {
 +    if (ret  0  dev != NULL)
         if (qemuDomainSetDeviceOwnership(dom-conn, driver, dev, 1)  0)
             VIR_WARN0(Fail to restore disk device ownership);
 -        virDomainDeviceDefFree(dev);
 -    }
 +    virDomainDeviceDefFree(dev);
 +
     if (vm)
         virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
 --
 1.6.6.638.g2bc54

NACK. This will probably result in a segfault because you are freeing
memory that is still in use.

Yes the toplevel dev leaks here, but for example
qemudDomainAttachNetDevice some lines above takes parts from the dev
struct an assigns them to other structs _without_ copying them. I
found this leak some time ago too, but gave up on fixing it as I
noticed how entangled this code is.

Matthias

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

[libvirt] [PATCH] Fix QEMU driver custom domain status XML extensions

2010-01-18 Thread Daniel P. Berrange

Invoking the virConnectGetCapabilities() method causes the QEMU
driver to rebuild its internal capabilities object. Unfortunately
it was forgetting to register the custom domain status XML hooks
again.

To avoid this kind of error in the future, the code which builds
capabilities is refactored into one single method, which can be
called from all locations, ensuring reliable rebuilds.

* src/qemu/qemu_driver.c: Fix rebuilding of capabilities XML and
  guarentee it is always consistent
---
 src/qemu/qemu_driver.c |  110 +++-
 1 files changed, 53 insertions(+), 57 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2d80774..9a3ddfb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -892,34 +892,6 @@ qemuReconnectDomains(struct qemud_driver *driver)
 
 
 static int
-qemudSecurityCapsInit(virSecurityDriverPtr secdrv,
-  virCapsPtr caps)
-{
-const char *doi, *model;
-
-doi = virSecurityDriverGetDOI(secdrv);
-model = virSecurityDriverGetModel(secdrv);
-
-caps-host.secModel.model = strdup(model);
-if (!caps-host.secModel.model) {
-virReportOOMError(NULL);
-return -1;
-}
-
-caps-host.secModel.doi = strdup(doi);
-if (!caps-host.secModel.doi) {
-virReportOOMError(NULL);
-return -1;
-}
-
-VIR_DEBUG(Initialized caps for security driver \%s\ with 
-  DOI \%s\, model, doi);
-
-return 0;
-}
-
-
-static int
 qemudSecurityInit(struct qemud_driver *qemud_drv)
 {
 int ret;
@@ -940,15 +912,52 @@ qemudSecurityInit(struct qemud_driver *qemud_drv)
 qemud_drv-securityDriver = security_drv;
 
 VIR_INFO(Initialized security driver %s, security_drv-name);
-
-/*
- * Add security policy host caps now that the security driver is
- * initialized.
- */
-return qemudSecurityCapsInit(security_drv, qemud_drv-caps);
+return 0;
 }
 
 
+static virCapsPtr
+qemuCreateCapabilities(virCapsPtr oldcaps,
+   virSecurityDriverPtr secDriver)
+{
+virCapsPtr caps;
+
+/* Basic host arch / guest machine capabilities */
+if (!(caps = qemudCapsInit(oldcaps))) {
+virReportOOMError(NULL);
+return NULL;
+}
+
+/* Domain XML parser hooks */
+caps-privateDataAllocFunc = qemuDomainObjPrivateAlloc;
+caps-privateDataFreeFunc = qemuDomainObjPrivateFree;
+caps-privateDataXMLFormat = qemuDomainObjPrivateXMLFormat;
+caps-privateDataXMLParse = qemuDomainObjPrivateXMLParse;
+
+
+/* Security driver data */
+if (secDriver) {
+const char *doi, *model;
+
+doi = virSecurityDriverGetDOI(secDriver);
+model = virSecurityDriverGetModel(secDriver);
+
+if (!(caps-host.secModel.model = strdup(model)))
+goto no_memory;
+if (!(caps-host.secModel.doi = strdup(doi)))
+goto no_memory;
+
+VIR_DEBUG(Initialized caps for security driver \%s\ with 
+  DOI \%s\, model, doi);
+}
+
+return caps;
+
+no_memory:
+virReportOOMError(NULL);
+virCapabilitiesFree(caps);
+return NULL;
+}
 
 /**
  * qemudStartup:
@@ -1074,13 +1083,12 @@ qemudStartup(int privileged) {
  virStrerror(-rc, buf, sizeof(buf)));
 }
 
-if ((qemu_driver-caps = qemudCapsInit(NULL)) == NULL)
-goto out_of_memory;
+if (qemudSecurityInit(qemu_driver)  0)
+goto error;
 
-qemu_driver-caps-privateDataAllocFunc = qemuDomainObjPrivateAlloc;
-qemu_driver-caps-privateDataFreeFunc = qemuDomainObjPrivateFree;
-qemu_driver-caps-privateDataXMLFormat = qemuDomainObjPrivateXMLFormat;
-qemu_driver-caps-privateDataXMLParse = qemuDomainObjPrivateXMLParse;
+if ((qemu_driver-caps = qemuCreateCapabilities(NULL,
+
qemu_driver-securityDriver)) == NULL)
+goto error;
 
 if ((qemu_driver-activePciHostdevs = pciDeviceListNew(NULL)) == NULL)
 goto error;
@@ -1104,10 +1112,6 @@ qemudStartup(int privileged) {
 }
 }
 
-if (qemudSecurityInit(qemu_driver)  0) {
-goto error;
-}
-
 /* If hugetlbfs is present, then we need to create a sub-directory within
  * it, since we can't assume the root mount point has permissions that
  * will let our spawned QEMU instances use it.
@@ -3255,15 +3259,12 @@ static char *qemudGetCapabilities(virConnectPtr conn) {
 char *xml = NULL;
 
 qemuDriverLock(driver);
-if ((caps = qemudCapsInit(qemu_driver-caps)) == NULL)
-goto no_memory;
 
-caps-privateDataAllocFunc = qemuDomainObjPrivateAlloc;
-caps-privateDataFreeFunc = qemuDomainObjPrivateFree;
-
-if (qemu_driver-securityDriver 
-qemudSecurityCapsInit(qemu_driver-securityDriver, caps)  0)
-goto no_memory;
+if ((caps = qemuCreateCapabilities(qemu_driver-caps,
+   qemu_driver-securityDriver)) == NULL) {
+

Re: [libvirt] [PATCH] qemu_driver: don't leak a virDomainDeviceDef buffer

2010-01-18 Thread Jim Meyering

Matthias Bolte wrote:
 2010/1/18 Jim Meyering j...@meyering.net:
 At first I was going to call virDomainDeviceDefFree only if (dev),
 but saw that it handles a NULL dev just fine, so it's better to skip
 the test altogether, just as we do for many other free-like functions.

 From ea8511d709492f5cdc152a1eaccbccd05f036648 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 18 Jan 2010 16:55:36 +0100
 Subject: [PATCH] qemu_driver: don't leak a virDomainDeviceDef buffer

 * src/qemu/qemu_driver.c (qemudDomainAttachDevice): Don't leak dev.
...

 NACK. This will probably result in a segfault because you are freeing
 memory that is still in use.

 Yes the toplevel dev leaks here, but for example
 qemudDomainAttachNetDevice some lines above takes parts from the dev
 struct an assigns them to other structs _without_ copying them.

Thanks.
That is nastily unintuitive and sounds a lot like a bug.
I hope it's on the list of things to be fixed by Dan's patch.

 I found this leak some time ago too, but gave up on fixing it as I
 noticed how entangled this code is.

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

[libvirt] [PATCH] storage_conf: plug a leak on OOM error path

2010-01-18 Thread Jim Meyering

Obvious, for a change...

From 964f24e0724eda6bf12d115318ea9576bbd91f10 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 18 Jan 2010 18:40:13 +0100
Subject: [PATCH] storage_conf: plug a leak on OOM error path

* src/conf/storage_conf.c (virStoragePoolSourceListNewSource):
Free just-allocated source upon VIR_REALLOC_N failure.
---
 src/conf/storage_conf.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 0aefa06..ea49531 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1,7 +1,7 @@
 /*
  * storage_conf.c: config handling for storage driver
  *
- * Copyright (C) 2006-2009 Red Hat, Inc.
+ * Copyright (C) 2006-2010 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -1695,6 +1695,7 @@ virStoragePoolSourceListNewSource(virConnectPtr conn,
 }

 if (VIR_REALLOC_N(list-sources, list-nsources+1)  0) {
+VIR_FREE(source);
 virReportOOMError(conn);
 return NULL;
 }
--
1.6.6.638.g2bc54

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

Re: [libvirt] [PATCH] xen_driver: don't leak a parsed-config buffer

2010-01-18 Thread Matthias Bolte

2010/1/18 Jim Meyering j...@meyering.net:
 Another leak:

 From 10a428a2ed1ddbe4da2b4fbd7690da5a0fe0420b Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 18 Jan 2010 18:02:17 +0100
 Subject: [PATCH] xen_driver: don't leak a parsed-config buffer

 * src/xen/xen_driver.c (xenUnifiedDomainXMLFromNative): Also
 free conf before returning.
 ---
  src/xen/xen_driver.c |    4 +++-
  1 files changed, 3 insertions(+), 1 deletions(-)

 diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
 index 4911c9e..72f56ae 100644
 --- a/src/xen/xen_driver.c
 +++ b/src/xen/xen_driver.c
 @@ -1,7 +1,7 @@
  /*
  * xen_driver.c: Unified Xen driver.
  *
 - * Copyright (C) 2007, 2008, 2009 Red Hat, Inc.
 + * Copyright (C) 2007-2010 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
 @@ -1199,6 +1199,8 @@ xenUnifiedDomainXMLFromNative(virConnectPtr conn,

  cleanup:
     virDomainDefFree(def);
 +    if (conf)
 +        virConfFree(conf);
     return ret;
  }

 --
 1.6.6.638.g2bc54


ACK.

Matthias

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

[libvirt] virsh list after libvirt restart forgets existing guests (0.7.5)

2010-01-18 Thread Thomas Sjolshagen

After the upgrade to 0.7.5 from the preview repository, I've noticed that a 
restart of libvirt does not appear to re-acquire any guests already running 
on the system.

# virsh list
Compiled against library: libvir 0.7.5
Using library: libvir 0.7.5
Using API: QEMU 0.7.5
Running hypervisor: QEMU 0.12.1

# ps -ef | grep qemu-kvm
qemu  1445 1  0 Jan17 ?00:03:32 /usr/bin/qemu-kvm -S -M pc-0.11 
-enable-kvm -m 512 -smp 1 -name imap [SNIP]
qemu  1516 1  0 Jan17 ?00:04:11 /usr/bin/qemu-kvm -S -M pc-0.11 
-cpu qemu32 -enable-kvm -m 512 -smp 1 -name mail [SNIP]
qemu 21409 1 16 Jan16 ?07:29:32 /usr/bin/qemu-kvm -S -M pc-0.11 
-enable-kvm -m 1640 -smp 1 -name imap2-cluster [SNIP]

# virsh list
 Id Name State
--
  1 imap running
  2 mail running

Additionally, something doesn't appear to be getting either parsed right or set 
correctly when the guest is started as the 
/var/log/libvirt/qemu/guestname.log file contains: 
Option 'ipv4: Use 'on' or 'off' 
Failed to parse yes for dummy.ipv4


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

Re: [libvirt] virsh list after libvirt restart forgets existing guests (0.7.5)

2010-01-18 Thread Daniel P. Berrange

On Mon, Jan 18, 2010 at 01:03:12PM -0500, Thomas Sjolshagen wrote:
 After the upgrade to 0.7.5 from the preview repository, I've 
 noticed that a restart of libvirt does not appear to re-acquire
 any guests already running on the system.

See these two links

  http://www.redhat.com/archives/libvir-list/2010-January/msg00545.html
  https://bugzilla.redhat.com/show_bug.cgi?id=550400

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] hvm/x86_64 combination not allowed

2010-01-18 Thread Steve Brown

 $ sudo virsh capabilities
 capabilities
   host
     cpu
       archx86_64/arch
     /cpu
     migration_features
       live/
       uri_transports
         uri_transporttcp/uri_transport
       /uri_transports
     /migration_features
   /host
   guest
     os_typehvm/os_type
     arch name='i686'
       wordsize32/wordsize
       emulator/usr/bin/qemu/emulator
       machinepc-0.11/machine
       machine canonical='pc-0.11'pc/machine
       machinepc-0.10/machine
       machineisapc/machine
       domain type='qemu'
       /domain
     /arch
     features
       pae/
       nonpae/
       acpi default='on' toggle='yes'/
       apic default='on' toggle='no'/
     /features
   /guest
 /capabilities

 Ok, this confirms your host OS is x86_64, but it only shows a single
 guest entry for i386. This is because the 'qemu' binar is the 32-bit
 emulator. To make x86_64 guests work, you need to install the
 qemu-system-x86_64 binary, or a KVM binary called 'kvm' or 'qemu-kvm'.

 Once those are installed, you should see another guest appear in
 that capabilities XML

Unfortunately, I already have the 64bit emulator installed:

$ which qemu-system-x86_64
/usr/local/bin/qemu-system-x86_64
$ ls -l /usr/local/bin/qemu-*
-rwxr-xr-x 1 root root  217184 Jan 14 14:00 /usr/local/bin/qemu-img
-rwxr-xr-x 1 root root  229120 Jan 14 14:00 /usr/local/bin/qemu-io
-rwxr-xr-x 1 root root  210968 Jan 14 14:00 /usr/local/bin/qemu-nbd
-rwxr-xr-x 1 root root 2323328 Jan 14 14:01 /usr/local/bin/qemu-system-x86_64

This was done by building the qemu-kvm-0.12.1.1 package from source.
I am able to run this vm just fine using the qemu-system-x86_64
binary.  I've tried rebuilding libvirt several times to make it aware
that the proper binaries are installed, but it can't seem to find them
for some reason.

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

Re: [libvirt] Current cpu and memory usage for Host and Domains

2010-01-18 Thread su disheng

On Sun, Jan 17, 2010 at 5:02 AM, Daniel P. Berrange berra...@redhat.comwrote:

 On Fri, Jan 15, 2010 at 11:52:09AM -0800, su disheng wrote:
  Seem there is no way to get host CPU usage in libvirt API? I can get
  freeMemory by  virNodeGetFreeMemory, but no cpu usage API.
  How about add a new api for it? The host CPU usage is also important for
 VM
  creation and migration decision, like the free memory.
  Yes, I know, I can got it from top...:)

 We've historically thought that monitoring of the host OS is out of
 scope of the libvirt API, since there are already a huge variety of
 monitoring services / APIs available for people.  I wonder if we need
 to reconsider that now we have the VMWare drivers though - apps using
 libvirt don't get to installl stuff in VMWare host OS, so the only
 access to some of this host CPU data is probably via the VMWare APIs.
 Which in turn suggests we'd need to expose some minimal hosting API
 for this in libvirt


Like this happened in the future, one APIs for both host and guest.
Especially, for KVM,
there is no much difference to get the both host and guest, cpu/disk/nic
statistic data.


 FWIW, virt-top/virt-manager calculates host CPU usage by summing the usage
 of all guests over a fixed time slice. This will be an underestimate of
 true host utilization though, since it can't see CPU usage for non-guest
 related processes


For Xen like hypervisors, by summing the usage of all guests is OK, but for
KVM, it's
better to include other non-guest related processes.


 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/http://search.cpan.org/%7Edanberr/:|
 |: 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] Cannot use console with 0.7.5, error: internal error no assigned pty for device serial0

2010-01-18 Thread Guido Günther

On Sun, Jan 17, 2010 at 01:27:21PM +0100, Matthias Bolte wrote:
 2010/1/17 Guido Günther a...@sigxcpu.org:
  On Sat, Jan 16, 2010 at 06:24:28PM +0100, Marc Haber wrote:
  Hi,
 
  On Fri, Jan 15, 2010 at 03:13:25PM +0100, Marc Haber wrote:
   $ dpkg --list *kvm*
   Desired=Unknown/Install/Remove/Purge/Hold
   | 
   Status=Not/Inst/Cfg-files/Unpacked/Failed-cfg/Half-inst/trig-aWait/Trig-pend
   |/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
   ||/ Name           Version        Description
   +++-==-==-
   ii  kvm            72+dfsg-2      Full virtualization on x86 hardware
 
  This turned out to be the problem, libvirt didn't handle KVM 72's
  output correctly. photron helped identifying this issue on IRC, which
  I really appreciate, and there is a patch available to fix libvirt
  0.7.5 with old KVM again.
  Can you point me to this patch? Libvirt should be able to run with kvm
  72 too.
   -- Guido
 
 
 See https://www.redhat.com/archives/libvir-list/2010-January/msg00470.html
Will be part of the next Debian package. Thanks!
 -- Guido

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

[libvirt] virt-manager network properties

2010-01-18 Thread C.J. Adams-Collier

Hey Dan, list,

It seems to me that virt-manager could do with more information about
the virtual network devices, the DHCP leases on them, maybe a view of
the arp table, etc.

To this end, I've abused Dan's .glade file a bit and put some stub code
in virtManger/host.py

Could I get some comments and some recommendations for next steps?  It's
been a while since I've done any glade'ing...

Cheers,

C.J.



diff -r e95681a690fd src/virtManager/host.py
--- a/src/virtManager/host.py	Thu Jan 14 10:09:52 2010 -0500
+++ b/src/virtManager/host.py	Mon Jan 18 13:45:41 2010 -0800
@@ -100,6 +100,7 @@
 on_menu_help_contents_activate: self.show_help,
 on_net_add_clicked: self.add_network,
 on_net_delete_clicked: self.delete_network,
+on_net_properties_clicked: self.network_properties,
 on_net_stop_clicked: self.stop_network,
 on_net_start_clicked: self.start_network,
 on_net_autostart_toggled: self.net_autostart_changed,
@@ -292,6 +293,16 @@
 self.err.show_err(_(Error deleting network: %s) % str(e),
   .join(traceback.format_exc()))
 
+def network_properties(self, src):
+net = self.current_network()
+if net is None:
+return
+
+result = self.err.yes_no(_(Are you sure you see the properties 
+   of network %s?) % net.get_name())
+if not result:
+return
+
 def start_network(self, src):
 net = self.current_network()
 if net is None:
diff -r e95681a690fd src/vmm-host.glade
--- a/src/vmm-host.glade	Thu Jan 14 10:09:52 2010 -0500
+++ b/src/vmm-host.glade	Mon Jan 18 13:45:41 2010 -0800
@@ -905,6 +905,27 @@
 property name=position3/property
   /packing
 /child
+child
+  widget class=GtkButton id=net-properties
+property name=visibleTrue/property
+property name=can_focusTrue/property
+property name=can_defaultTrue/property
+property name=receives_defaultTrue/property
+property name=tooltip translatable=yesNetwork Properties/property
+signal name=clicked handler=on_net_properties_clicked/
+child
+  widget class=GtkImage id=image8
+property name=visibleTrue/property
+property name=stockgtk-properties/property
+  /widget
+/child
+  /widget
+  packing
+property name=expandFalse/property
+property name=fillFalse/property
+property name=position4/property
+  /packing
+/child
   /widget
   packing
 property name=position0/property


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] hvm/x86_64 combination not allowed

2010-01-18 Thread Matthias Bolte

2010/1/18 Steve Brown sbrow...@gmail.com:
 $ sudo virsh capabilities
 capabilities
   host
     cpu
       archx86_64/arch
     /cpu
     migration_features
       live/
       uri_transports
         uri_transporttcp/uri_transport
       /uri_transports
     /migration_features
   /host
   guest
     os_typehvm/os_type
     arch name='i686'
       wordsize32/wordsize
       emulator/usr/bin/qemu/emulator
       machinepc-0.11/machine
       machine canonical='pc-0.11'pc/machine
       machinepc-0.10/machine
       machineisapc/machine
       domain type='qemu'
       /domain
     /arch
     features
       pae/
       nonpae/
       acpi default='on' toggle='yes'/
       apic default='on' toggle='no'/
     /features
   /guest
 /capabilities

 Ok, this confirms your host OS is x86_64, but it only shows a single
 guest entry for i386. This is because the 'qemu' binar is the 32-bit
 emulator. To make x86_64 guests work, you need to install the
 qemu-system-x86_64 binary, or a KVM binary called 'kvm' or 'qemu-kvm'.

 Once those are installed, you should see another guest appear in
 that capabilities XML

 Unfortunately, I already have the 64bit emulator installed:

 $ which qemu-system-x86_64
 /usr/local/bin/qemu-system-x86_64
 $ ls -l /usr/local/bin/qemu-*
 -rwxr-xr-x 1 root root  217184 Jan 14 14:00 /usr/local/bin/qemu-img
 -rwxr-xr-x 1 root root  229120 Jan 14 14:00 /usr/local/bin/qemu-io
 -rwxr-xr-x 1 root root  210968 Jan 14 14:00 /usr/local/bin/qemu-nbd
 -rwxr-xr-x 1 root root 2323328 Jan 14 14:01 /usr/local/bin/qemu-system-x86_64

 This was done by building the qemu-kvm-0.12.1.1 package from source.
 I am able to run this vm just fine using the qemu-system-x86_64
 binary.  I've tried rebuilding libvirt several times to make it aware
 that the proper binaries are installed, but it can't seem to find them
 for some reason.


libvirt expects the QEMU binaries in /usr/bin. e.g. it explicitly
checks for /usr/bin/qemu-system-x86_64. Try symlinking
/usr/local/bin/qemu-system-x86_64 to /usr/bin/qemu-system-x86_64.

I'm not sure why the paths are hardcoded instead of using the PATH to
find the QEMU binaries, like the storage driver does it already:

  virFindFileInPath(qemu-img)

Maybe Daniel can explain.

Matthias

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

Re: [libvirt] [PATCH] storage_conf: plug a leak on OOM error path

2010-01-18 Thread Matthias Bolte

2010/1/18 Jim Meyering j...@meyering.net:
 Obvious, for a change...

 From 964f24e0724eda6bf12d115318ea9576bbd91f10 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 18 Jan 2010 18:40:13 +0100
 Subject: [PATCH] storage_conf: plug a leak on OOM error path

 * src/conf/storage_conf.c (virStoragePoolSourceListNewSource):
 Free just-allocated source upon VIR_REALLOC_N failure.
 ---
  src/conf/storage_conf.c |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index 0aefa06..ea49531 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c
 @@ -1,7 +1,7 @@
  /*
  * storage_conf.c: config handling for storage driver
  *
 - * Copyright (C) 2006-2009 Red Hat, Inc.
 + * Copyright (C) 2006-2010 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
 @@ -1695,6 +1695,7 @@ virStoragePoolSourceListNewSource(virConnectPtr conn,
     }

     if (VIR_REALLOC_N(list-sources, list-nsources+1)  0) {
 +        VIR_FREE(source);
         virReportOOMError(conn);
         return NULL;
     }
 --
 1.6.6.638.g2bc54


ACK.

Matthias

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

[libvirt] Libvirt guide draft published on libvirt.org

2010-01-18 Thread David Jorm

I am a tech writer who recently joined the Red Hat team. I have been tasked 
with assisting in the improvement of libvirt documentation where possible and 
co-ordinating the development of the libvirt Application Development Guide. The 
guide was previously in the hands of Dani Coulson, who has since left Red Hat. 
She got it to a draft stage with a skeletal structure, but as far as I can tell 
nothing ever reached a publishable state. I've picked up where she left off and 
re-built the latest guide from the DocBook XML in git. It is now up at:

http://libvirt.org/guide/

If you look in the guide, you will notice an awful lot of TBD stubs. 
Contributions to fill these would be greatly appreciated - please email them to 
me directly. I will chase up with the people who were originally nominated as 
the responsible parties to try and get some content to flesh out the guide.

I don't have a lot of spare temporal bandwidth at the moment, but if there are 
any docs-related BZs or libvirt issues, feel free to push them my way and i'll 
do what I can. I think i've made every mistake possible so far in submitting 
patches, so I know the process by virtue of what-not-to-do.

Thanks
David

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

Re: [libvirt] [PATCH v3] Implement CPU topology support for QEMU driver

2010-01-18 Thread Matthias Bolte

2010/1/18 Jiri Denemark jdene...@redhat.com:
 QEMU's command line equivalent for the following domain XML fragment
    vcpus2/vcpus
    cpu ...
        ...
        topology sockets='1' cores='2', threads='1'/
    /cpu

 is

    -smp 2,sockets=1,cores=2,threads=1

 This syntax was introduced in QEMU-0.12.

 Version 2 changes:
 - -smp argument build split into a separate function
 - always add ,sockets=S,cores=C,threads=T to -smp if qemu supports it
 - use qemuParseCommandLineKeywords for command line parsing

 Version 3 changes:
 - ADD_ARG_LIT = ADD_ARG and line reordering in qemudBuildCommandLine
 - rebased

 Signed-off-by: Jiri Denemark jdene...@redhat.com
 ---
  src/qemu/qemu_conf.c |  157 -
  src/qemu/qemu_conf.h |    3 +-
  2 files changed, 143 insertions(+), 17 deletions(-)

 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index 024b2ba..1fbb86a 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
[...]
 @@ -4714,11 +4771,84 @@ syntax:
  no_memory:
     virReportOOMError(conn);
  error:
 -    virCPUDefFree(cpu);
     return -1;
  }


 +static int
 +qemuParseCommandLineSmp(virConnectPtr conn,
 +                        virDomainDefPtr dom,
 +                        const char *val)
 +{
 +    unsigned int sockets = 0;
 +    unsigned int cores = 0;
 +    unsigned int threads = 0;
 +    int i;
 +    int nkws;
 +    char **kws;
 +    char **vals;
 +    int n;
 +    char *end;
 +    int ret;
 +
 +    nkws = qemuParseCommandLineKeywords(conn, val, kws, vals, 1);

What's the purpose of the 1 here? qemuParseCommandLineKeywords takes 4
parameters, you pass 5. This breaks compilation for me.

Matthias

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

Re: [libvirt] hvm/x86_64 combination not allowed

2010-01-18 Thread Laine Stump


On 01/18/2010 04:59 PM, Matthias Bolte wrote:

2010/1/18 Steve Brownsbrow...@gmail.com:
   

$ sudo virsh capabilities
capabilities
   host
 cpu
   archx86_64/arch
 /cpu
 migration_features
   live/
   uri_transports
 uri_transporttcp/uri_transport
   /uri_transports
 /migration_features
   /host
   guest
 os_typehvm/os_type
 arch name='i686'
   wordsize32/wordsize
   emulator/usr/bin/qemu/emulator
   machinepc-0.11/machine
   machine canonical='pc-0.11'pc/machine
   machinepc-0.10/machine
   machineisapc/machine
   domain type='qemu'
   /domain
 /arch
 features
   pae/
   nonpae/
   acpi default='on' toggle='yes'/
   apic default='on' toggle='no'/
 /features
   /guest
/capabilities
 

Ok, this confirms your host OS is x86_64, but it only shows a single
guest entry for i386. This is because the 'qemu' binar is the 32-bit
emulator. To make x86_64 guests work, you need to install the
qemu-system-x86_64 binary, or a KVM binary called 'kvm' or 'qemu-kvm'.

Once those are installed, you should see anotherguest  appear in
that capabilities XML
   

Unfortunately, I already have the 64bit emulator installed:

$ which qemu-system-x86_64
/usr/local/bin/qemu-system-x86_64
$ ls -l /usr/local/bin/qemu-*
-rwxr-xr-x 1 root root  217184 Jan 14 14:00 /usr/local/bin/qemu-img
-rwxr-xr-x 1 root root  229120 Jan 14 14:00 /usr/local/bin/qemu-io
-rwxr-xr-x 1 root root  210968 Jan 14 14:00 /usr/local/bin/qemu-nbd
-rwxr-xr-x 1 root root 2323328 Jan 14 14:01 /usr/local/bin/qemu-system-x86_64

This was done by building the qemu-kvm-0.12.1.1 package from source.
I am able to run this vm just fine using the qemu-system-x86_64
binary.  I've tried rebuilding libvirt several times to make it aware
that the proper binaries are installed, but it can't seem to find them
for some reason.

 

libvirt expects the QEMU binaries in /usr/bin. e.g. it explicitly
checks for /usr/bin/qemu-system-x86_64. Try symlinking
/usr/local/bin/qemu-system-x86_64 to /usr/bin/qemu-system-x86_64.
   


That's not enough. I have a locally-built qemu-system-x86_64 installed 
in /usr/bin, and the only way I could get it to work properly was copy 
/usr/bin/qemu-system-x86_64 to /usr/bin/qemu-kvm (a symlink would likely 
do the job just as well). There is a bit of code just doesn't happen 
unless /usr/bin/qemu-kvm or /usr/bin/kvm exist and are executable, 
although I have to say that once I solved my own problem by making a 
copy of the file with the proper name, I stopped trying to understood 
exactly what was happening in that code :-)


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

42 matches

Mail list logo