[libvirt] [sandbox][PATCH] Fix nits in virt-sandbox-service when raise ValueError

2013-05-28 Thread Wayne Sun
Put error msg in list when raise ValueError.

This fix is for bug:
[virt-sandbox-service] execute command with unsupported URI error msg is not 
right
https://bugzilla.redhat.com/show_bug.cgi?id=967705

Signed-off-by: Wayne Sun g...@redhat.com
---
 bin/virt-sandbox-service |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 4496b29..1db3c09 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -878,7 +878,7 @@ def sandbox_list(args):
 def sandbox_reload(args):
 config = read_config(args.name)
 if isinstance(config, gi.repository.LibvirtSandbox.ConfigServiceGeneric):
-raise ValueError(_(Generic Containers do not support reload))
+raise ValueError([_(Generic Containers do not support reload)])
 container = SystemdContainer(uri = args.uri, config = config)
 container.reload(args.unitfiles)
 
@@ -931,7 +931,7 @@ def fullpath(cmd):
 
 def execute(args):
 if args.uri != lxc:///:
-raise ValueError(_(Can only execute commands inside of linux 
containers.))
+raise ValueError([_(Can only execute commands inside of linux 
containers.)])
 
 myexec = [ virsh, -c, args.uri, lxc-enter-namespace ]
 #myexec = [ virt-sandbox-service-util, execute ]
-- 
1.7.1

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


Re: [libvirt] [PATCH] libvirt supports Guest Panicked

2013-05-28 Thread chenfan
On Mon, 2013-05-27 at 14:28 +0200, Viktor Mihajlovski wrote:
 On 05/27/2013 10:41 AM, Chen Fan wrote:
  From: chenfan chen.fan.f...@cn.fujitsu.com
  @@ -3185,6 +3194,9 @@ int qemuMonitorVMStatusToPausedReason(const char 
  *status)
case QEMU_MONITOR_VM_STATUS_WATCHDOG:
return VIR_DOMAIN_PAUSED_WATCHDOG;
 
  +case QEMU_MONITOR_VM_STATUS_GUEST_PANICKED:
  +return VIR_DOMAIN_PAUSED_GUEST_PANICKED;
  +
/* unreachable from this point on */
case QEMU_MONITOR_VM_STATUS_LAST:
;
 I just have crashed the guest while libvirtd wasn't running,
 after a restart, virsh domstate --reason always reports
 paused (guest panicked) while I would expect crashed (guest panicked).
 Either QEMU is reporting a bogus state or the state computation
 is flawed somewhere...
 
Then I will fix this bug. and remain on_crash default behavior in the
XML.
Thanks for your comments.

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


[libvirt] [PATCH v2] libvirt supports Guest Panicked

2013-05-28 Thread Chen Fan
From: ChenFan chen.fan.f...@cn.fujitsu.com

This patch implements qemu_driver supporting guest panicked.
we crashed the guest while libvirt isn't running, then restart libvirtd, we 
change the domain state to 'crashed'.

---
 examples/domain-events/events-c/event-test.c |  10 +++
 include/libvirt/libvirt.h.in |  16 +
 src/conf/domain_conf.c   |  12 ++--
 src/qemu/qemu_monitor.c  |  14 +++-
 src/qemu/qemu_monitor.h  |   5 ++
 src/qemu/qemu_monitor_json.c |   7 ++
 src/qemu/qemu_process.c  | 103 ++-
 tools/virsh-domain-monitor.c |   8 +++
 8 files changed, 169 insertions(+), 6 deletions(-)

diff --git a/examples/domain-events/events-c/event-test.c 
b/examples/domain-events/events-c/event-test.c
index eeff50f..1b425fb 100644
--- a/examples/domain-events/events-c/event-test.c
+++ b/examples/domain-events/events-c/event-test.c
@@ -93,6 +93,9 @@ const char *eventToString(int event) {
 case VIR_DOMAIN_EVENT_PMSUSPENDED:
 ret = PMSuspended;
 break;
+case VIR_DOMAIN_EVENT_CRASHED:
+ret = Crashed;
+break;
 }
 return ret;
 }
@@ -209,6 +212,13 @@ static const char *eventDetailToString(int event, int 
detail) {
 break;
 }
 break;
+case VIR_DOMAIN_EVENT_CRASHED:
+   switch ((virDomainEventCrashedDetailType) detail) {
+   case VIR_DOMAIN_EVENT_CRASHED_PANICKED:
+   ret = Panicked;
+   break;
+   }
+   break;
 }
 return ret;
 }
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 1804c93..56c6c5c 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -155,6 +155,7 @@ typedef enum {
 VIR_DOMAIN_RUNNING_SAVE_CANCELED = 7,   /* returned from failed save 
process */
 VIR_DOMAIN_RUNNING_WAKEUP = 8,  /* returned from pmsuspended due to
wakeup event */
+VIR_DOMAIN_RUNNING_CRASHED = 9,  /* resumed from crashed */
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_RUNNING_LAST
@@ -180,6 +181,7 @@ typedef enum {
 VIR_DOMAIN_PAUSED_FROM_SNAPSHOT = 7, /* paused after restoring from 
snapshot */
 VIR_DOMAIN_PAUSED_SHUTTING_DOWN = 8, /* paused during shutdown process */
 VIR_DOMAIN_PAUSED_SNAPSHOT = 9,  /* paused while creating a snapshot */
+VIR_DOMAIN_PAUSED_GUEST_PANICKED = 10, /* paused due to a guest panicked 
event */
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_PAUSED_LAST
@@ -189,6 +191,7 @@ typedef enum {
 typedef enum {
 VIR_DOMAIN_SHUTDOWN_UNKNOWN = 0,/* the reason is unknown */
 VIR_DOMAIN_SHUTDOWN_USER = 1,   /* shutting down on user request */
+VIR_DOMAIN_SHUTDOWN_CRASHED = 2,/* domain crashed */
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_SHUTDOWN_LAST
@@ -212,6 +215,7 @@ typedef enum {
 
 typedef enum {
 VIR_DOMAIN_CRASHED_UNKNOWN = 0, /* crashed for unknown reason */
+VIR_DOMAIN_CRASHED_PANICKED = 1, /* domain panicked */
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_CRASHED_LAST
@@ -3319,6 +3323,7 @@ typedef enum {
 VIR_DOMAIN_EVENT_STOPPED = 5,
 VIR_DOMAIN_EVENT_SHUTDOWN = 6,
 VIR_DOMAIN_EVENT_PMSUSPENDED = 7,
+VIR_DOMAIN_EVENT_CRASHED = 8,
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_EVENT_LAST
@@ -3450,6 +3455,17 @@ typedef enum {
 #endif
 } virDomainEventPMSuspendedDetailType;
 
+/*
+ * Details about the 'crashed' lifecycle event
+ */
+typedef enum {
+VIR_DOMAIN_EVENT_CRASHED_PANICKED = 0, /* Guest was panicked */
+
+#ifdef VIR_ENUM_SENTINELS
+VIR_DOMAIN_EVENT_CRASHED_LAST
+#endif
+} virDomainEventCrashedDetailType;
+
 /**
  * virConnectDomainEventCallback:
  * @conn: virConnect connection
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a9656af..f577fd4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -642,7 +642,8 @@ VIR_ENUM_IMPL(virDomainRunningReason, 
VIR_DOMAIN_RUNNING_LAST,
   unpaused,
   migration canceled,
   save canceled,
-  wakeup)
+  wakeup,
+  from crashed)
 
 VIR_ENUM_IMPL(virDomainBlockedReason, VIR_DOMAIN_BLOCKED_LAST,
   unknown)
@@ -657,11 +658,13 @@ VIR_ENUM_IMPL(virDomainPausedReason, 
VIR_DOMAIN_PAUSED_LAST,
   watchdog,
   from snapshot,
   shutdown,
-  snapshot)
+  snapshot,
+  guest panicked)
 
 VIR_ENUM_IMPL(virDomainShutdownReason, VIR_DOMAIN_SHUTDOWN_LAST,
   unknown,
-  user)
+  user,
+  crashed)
 
 VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST,
   unknown,
@@ -674,7 +677,8 @@ VIR_ENUM_IMPL(virDomainShutoffReason, 
VIR_DOMAIN_SHUTOFF_LAST,
   from snapshot)
 
 

[libvirt] [PATCH 00/11] Support CHAP authentication for iscsi pool

2013-05-28 Thread Osier Yang
The XMLs like (auth type='chap' login='foo' passwd='kudo'/) was
introduced long ago, but it's never used for any pool backend. This
implements the support first (See 6/11 for details), and based on it,
using secret object for the authentication is added too. E.g.

auth type='chap' username='foo'
  secret uuid='48dcd4a4-b25f-4fc6-8874-84797c6e3678'/
/auth

Osier Yang (11):
  storage: Refactor the rng schema for storage pool auth
  storage: Support username for chap type auth
  storage: Add a struct for auth secret
  storage: Introduce XMLs to use secret object for pool auth
  storage: Output auth type before username
  storage: Support chap authentication for iscsi pool
  storage: Support to use secret object for iscsi chap auth
  storage: Update docs/formatsecret.html
  storage: Use the internal API to get the secret value instead
  storage: Improve the pool auth type parsing and formating
  Storage: Fix the indention of rbd test file

 docs/formatsecret.html.in  |  10 +-
 docs/schemas/storagepool.rng   |  51 +++---
 src/conf/storage_conf.c| 179 ++---
 src/conf/storage_conf.h|  28 +++-
 src/storage/storage_backend_iscsi.c| 114 -
 src/storage/storage_backend_rbd.c  |  13 +-
 .../storagepoolxml2xmlin/pool-iscsi-auth-login.xml |  17 ++
 .../pool-iscsi-auth-secret.xml |  19 +++
 .../pool-iscsi-auth-username.xml   |  17 ++
 tests/storagepoolxml2xmlin/pool-iscsi-auth.xml |  17 --
 tests/storagepoolxml2xmlin/pool-rbd.xml|   2 +-
 .../pool-iscsi-auth-login.xml  |  20 +++
 .../pool-iscsi-auth-secret.xml |  22 +++
 .../pool-iscsi-auth-username.xml   |  20 +++
 tests/storagepoolxml2xmlout/pool-iscsi-auth.xml|  20 ---
 .../pool-iscsi-vendor-product.xml  |   2 +-
 tests/storagepoolxml2xmlout/pool-rbd.xml   |   2 +-
 tests/storagepoolxml2xmltest.c |   3 +-
 18 files changed, 424 insertions(+), 132 deletions(-)
 create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-login.xml
 create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-secret.xml
 create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-username.xml
 delete mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-login.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-secret.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-username.xml
 delete mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth.xml

-- 
1.8.1.4

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


[libvirt] [PATCH 11/11] Storage: Fix the indention of rbd test file

2013-05-28 Thread Osier Yang
---
 tests/storagepoolxml2xmlin/pool-rbd.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/storagepoolxml2xmlin/pool-rbd.xml 
b/tests/storagepoolxml2xmlin/pool-rbd.xml
index 4ee8d56..056ba6e 100644
--- a/tests/storagepoolxml2xmlin/pool-rbd.xml
+++ b/tests/storagepoolxml2xmlin/pool-rbd.xml
@@ -5,7 +5,7 @@
 host name='localhost' port='6789'/
 host name='localhost' port='6790'/
 auth username='admin' type='ceph'
-   secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/
+  secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/
 /auth
   /source
 /pool
-- 
1.8.1.4

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


[libvirt] [PATCH 01/11] storage: Refactor the rng schema for storage pool auth

2013-05-28 Thread Osier Yang
The attributes/elements for auth type chap and ceph are complete
different, this separates them into groups.

And add interleave for login and passwd attributes of chap
type auth.
---
 docs/schemas/storagepool.rng | 42 ++
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 3c2158a..2595e37 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -280,28 +280,30 @@
 
   define name='sourceinfoauth'
 element name='auth'
-  attribute name='type'
-choice
-  valuechap/value
-  valueceph/value
-/choice
-  /attribute
   choice
-attribute name='login'
-  text/
-/attribute
-attribute name='username'
-  text/
-/attribute
+group
+  attribute name='type'
+valuechap/value
+  /attribute
+  interleave
+attribute name='login'
+  text/
+/attribute
+attribute name='passwd'
+  text/
+/attribute
+  /interleave
+/group
+group
+  attribute name='type'
+valueceph/value
+  /attribute
+  attribute name='username'
+text/
+  /attribute
+  ref name='sourceinfoauthsecret'/
+   /group
   /choice
-  optional
-attribute name='passwd'
-  text/
-/attribute
-  /optional
-  optional
-ref name='sourceinfoauthsecret'/
-  /optional
 /element
   /define
 
-- 
1.8.1.4

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


[libvirt] [PATCH 04/11] storage: Introduce XMLs to use secret object for pool auth

2013-05-28 Thread Osier Yang
Using plain password is still supported for back-compat reason.

Example XML:
auth type='chap' username='foo'
  secret uuid='48dcd4a4-b25f-4fc6-8874-84797c6e3678'/
/auth

* docs/schemas/storagepool.rng (Add sourceinfoauthsecret as a choice)
* src/conf/storage_conf.h (union passwd and virStoragePoolAuthSecret)
* src/conf/storage_conf.c (s/chap\.passwd/chap\.u\.passwd/;
   Add a helper virStoragePoolAuthDefFormat;
   Parse the secret XMLs for chap auth)
* tests/storagepoolxml2xmlin/pool-iscsi-auth-secret.xml: (New tests)
* tests/storagepoolxml2xmlout/pool-iscsi-auth-secret.xml: (Likewise)
---
 docs/schemas/storagepool.rng   |  9 ++-
 src/conf/storage_conf.c| 71 +++---
 src/conf/storage_conf.h| 11 +++-
 .../pool-iscsi-auth-secret.xml | 19 ++
 .../pool-iscsi-auth-secret.xml | 22 +++
 5 files changed, 107 insertions(+), 25 deletions(-)
 create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-secret.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-secret.xml

diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index ba6c741..386de1b 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -295,9 +295,12 @@
 text/
   /attribute
 /choice
-attribute name='passwd'
-  text/
-/attribute
+choice
+  attribute name='passwd'
+text/
+  /attribute
+  ref name='sourceinfoauthsecret'/
+/choice
   /interleave
 /group
 group
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 0047372..8f83bae 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -349,7 +349,10 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source)
 
 if (source-authType == VIR_STORAGE_POOL_AUTH_CHAP) {
 VIR_FREE(source-auth.chap.login);
-VIR_FREE(source-auth.chap.passwd);
+if (source-auth.chap.type == 
VIR_STORAGE_POOL_AUTH_CHAP_PLAIN_PASSWORD)
+VIR_FREE(source-auth.chap.u.passwd);
+else if (source-auth.chap.type == VIR_STORAGE_POOL_AUTH_CHAP_SECRET)
+VIR_FREE(source-auth.chap.u.secret.usage);
 }
 
 if (source-authType == VIR_STORAGE_POOL_AUTH_CEPHX) {
@@ -483,6 +486,8 @@ virStoragePoolDefParseAuthChap(xmlXPathContextPtr ctxt,
 {
 char *login = NULL;
 char *username = NULL;
+char *passwd = NULL;
+int rc;
 
 login = virXPathString(string(./auth/@login), ctxt);
 username = virXPathString(string(./auth/@username), ctxt);
@@ -507,10 +512,20 @@ virStoragePoolDefParseAuthChap(xmlXPathContextPtr ctxt,
 else if (username)
 auth-login = username;
 
-auth-passwd = virXPathString(string(./auth/@passwd), ctxt);
-if (auth-passwd == NULL) {
+passwd = virXPathString(string(./auth/@passwd), ctxt);
+rc = virStoragePoolDefParseAuthSecret(ctxt, auth-u.secret);
+
+if (passwd) {
+auth-type = VIR_STORAGE_POOL_AUTH_CHAP_PLAIN_PASSWORD;
+auth-u.passwd = passwd;
+} else if (rc == 0) {
+auth-type = VIR_STORAGE_POOL_AUTH_CHAP_SECRET;
+} else if (rc  0) {
+return -1;
+} else {
 virReportError(VIR_ERR_XML_ERROR, %s,
-   _(missing auth passwd attribute));
+   _(Either 'passwd' attribute or 'secret' element 
+ should be specified));
 return -1;
 }
 
@@ -1048,13 +1063,30 @@ virStoragePoolDefParseFile(const char *filename)
 return virStoragePoolDefParse(NULL, filename);
 }
 
+static void
+virStoragePoolAuthDefFormat(virBufferPtr buf,
+virStoragePoolAuthSecret secret)
+{
+char uuid[VIR_UUID_STRING_BUFLEN];
+
+virBufferAddLit(buf,   secret);
+if (secret.uuidUsable) {
+virUUIDFormat(secret.uuid, uuid);
+virBufferAsprintf(buf,  uuid='%s', uuid);
+}
+
+if (secret.usage != NULL) {
+virBufferAsprintf(buf,  usage='%s', secret.usage);
+}
+virBufferAddLit(buf, /\n);
+}
+
 static int
 virStoragePoolSourceFormat(virBufferPtr buf,
virStoragePoolOptionsPtr options,
virStoragePoolSourcePtr src)
 {
 int i, j;
-char uuid[VIR_UUID_STRING_BUFLEN];
 
 virBufferAddLit(buf,  source\n);
 if ((options-flags  VIR_STORAGE_POOL_SOURCE_HOST)  src-nhost) {
@@ -1129,26 +1161,23 @@ virStoragePoolSourceFormat(virBufferPtr buf,
 virBufferAsprintf(buf,format type='%s'/\n, format);
 }
 
-if (src-authType == VIR_STORAGE_POOL_AUTH_CHAP)
-virBufferAsprintf(buf,auth type='chap' username='%s' 
passwd='%s'/\n,
-  src-auth.chap.login,
-  src-auth.chap.passwd);
+if 

[libvirt] [PATCH 07/11] storage: Support to use secret object for iscsi chap auth

2013-05-28 Thread Osier Yang
Based on the plain password chap auth support, this gets
the secret value (password) with the secret driver methods,
and apply it for the iscsiadm update command.
---
 src/storage/storage_backend_iscsi.c | 56 +
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/src/storage/storage_backend_iscsi.c 
b/src/storage/storage_backend_iscsi.c
index 6a17b5a..516025a 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -35,6 +35,8 @@
 #include unistd.h
 #include sys/stat.h
 
+#include datatypes.h
+#include driver.h
 #include virerror.h
 #include storage_backend_scsi.h
 #include storage_backend_iscsi.h
@@ -42,6 +44,7 @@
 #include virlog.h
 #include virfile.h
 #include vircommand.h
+#include virobject.h
 #include virrandom.h
 #include virstring.h
 
@@ -746,10 +749,17 @@ cleanup:
 }
 
 static int
-virStorageBackendISCSISetAuth(virStoragePoolDefPtr def,
+virStorageBackendISCSISetAuth(virConnectPtr conn,
+  virStoragePoolDefPtr def,
   const char *portal,
   const char *target)
 {
+virSecretPtr secret = NULL;
+unsigned char *secret_value = NULL;
+const char *passwd = NULL;
+virStoragePoolAuthChap chap = def-source.auth.chap;
+int ret = -1;
+
 if (def-source.authType == VIR_STORAGE_POOL_AUTH_NONE)
 return 0;
 
@@ -759,6 +769,35 @@ virStorageBackendISCSISetAuth(virStoragePoolDefPtr def,
 return -1;
 }
 
+if (chap.type == VIR_STORAGE_POOL_AUTH_CHAP_SECRET) {
+if (chap.u.secret.uuidUsable)
+secret = virSecretLookupByUUID(conn, chap.u.secret.uuid);
+else
+secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_ISCSI,
+chap.u.secret.usage);
+
+if (secret) {
+size_t secret_size;
+secret_value = conn-secretDriver-secretGetValue(secret, 
secret_size, 0,
+  
VIR_SECRET_GET_VALUE_INTERNAL_CALL);
+if (!secret_value) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(could not get the value of the secret 
+ for username %s), chap.login);
+goto cleanup;
+}
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(username '%s' specified but secret not found),
+   chap.login);
+goto cleanup;
+}
+
+passwd = (const char *)secret_value;
+} else if (chap.type == VIR_STORAGE_POOL_AUTH_CHAP_PLAIN_PASSWORD) {
+passwd = chap.u.passwd;
+}
+
 if (virStorageBackendISCSINodeUpdate(portal,
  target,
  node.session.auth.authmethod,
@@ -770,14 +809,18 @@ virStorageBackendISCSISetAuth(virStoragePoolDefPtr def,
 virStorageBackendISCSINodeUpdate(portal,
  target,
  node.session.auth.password,
- def-source.auth.chap.u.passwd)  0)
-return -1;
+ passwd)  0)
+goto cleanup;
 
-return 0;
+ret = 0;
+cleanup:
+virObjectUnref(secret);
+VIR_FREE(secret_value);
+return ret;
 }
 
 static int
-virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED,
+virStorageBackendISCSIStartPool(virConnectPtr conn,
 virStoragePoolObjPtr pool)
 {
 char *portal = NULL;
@@ -817,7 +860,8 @@ virStorageBackendISCSIStartPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
   NULL, NULL)  0)
 goto cleanup;
 
-if (virStorageBackendISCSISetAuth(pool-def,
+if (virStorageBackendISCSISetAuth(conn,
+  pool-def,
   portal,
   pool-def-source.devices[0].path)  
0)
 goto cleanup;
-- 
1.8.1.4

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


[libvirt] [PATCH 08/11] storage: Update docs/formatsecret.html

2013-05-28 Thread Osier Yang
To mention that the secret type iscsi and ceph can be used
to storage pool too.
---
 docs/formatsecret.html.in | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in
index 50c9533..3e306b5 100644
--- a/docs/formatsecret.html.in
+++ b/docs/formatsecret.html.in
@@ -64,8 +64,9 @@
   a single codename/code element that specifies a usage name
   for the secret.  The Ceph secret can then be used by UUID or by
   this usage name via the codelt;authgt;/code element of
-  a a href=formatdomain.html#elementsDisksdisk
-  device/a. span class=sinceSince 0.9.7/span.
+  a a href=formatdomain.html#elementsDisksdisk device/a or
+  a a href=formatstorage.htmlstorage pool (rbd)/a.
+  span class=sinceSince 0.9.7/span.
 /p
 
 h3Usage type iscsi/h3
@@ -76,8 +77,9 @@
   a single codetarget/code element that specifies a usage name
   for the secret.  The iSCSI secret can then be used by UUID or by
   this usage name via the codelt;authgt;/code element of
-  a a href=formatdomain.html#elementsDisksdisk
-  device/a. span class=sinceSince 1.0.4/span.
+  a a href=formatdomain.html#elementsDisksdisk device/a or
+  a a href=formatstorage.htmlstorage pool (iscsi)/a.
+  span class=sinceSince 1.0.4/span.
 /p
 
 h2a name=exampleExample/a/h2
-- 
1.8.1.4

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


[libvirt] [PATCH 05/11] storage: Output auth type before username

2013-05-28 Thread Osier Yang
As a habit, type attribute is printed first.
---
 src/conf/storage_conf.c  | 2 +-
 tests/storagepoolxml2xmlout/pool-rbd.xml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 8f83bae..a876332 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1175,7 +1175,7 @@ virStoragePoolSourceFormat(virBufferPtr buf,
 }
 
 if (src-authType == VIR_STORAGE_POOL_AUTH_CEPHX) {
-virBufferAsprintf(buf,auth username='%s' type='ceph'\n,
+virBufferAsprintf(buf,auth type='ceph' username='%s'\n,
   src-auth.cephx.username);
 virStoragePoolAuthDefFormat(buf, src-auth.cephx.secret);
 virBufferAddLit(buf,/auth\n);
diff --git a/tests/storagepoolxml2xmlout/pool-rbd.xml 
b/tests/storagepoolxml2xmlout/pool-rbd.xml
index 309a6d9..4fe2fce 100644
--- a/tests/storagepoolxml2xmlout/pool-rbd.xml
+++ b/tests/storagepoolxml2xmlout/pool-rbd.xml
@@ -8,7 +8,7 @@
 namerbd/name
 host name='localhost' port='6789'/
 host name='localhost' port='6790'/
-auth username='admin' type='ceph'
+auth type='ceph' username='admin'
   secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/
 /auth
   /source
-- 
1.8.1.4

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


[libvirt] [PATCH 03/11] storage: Add a struct for auth secret

2013-05-28 Thread Osier Yang
This also abstracts the code for parsing the XMLs to use the secret
object as a helper.
---
 src/conf/storage_conf.c | 68 -
 src/conf/storage_conf.h | 14 ++
 2 files changed, 48 insertions(+), 34 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index c0bf084..0047372 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -439,6 +439,43 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
 }
 }
 
+static int
+virStoragePoolDefParseAuthSecret(xmlXPathContextPtr ctxt,
+ virStoragePoolAuthSecretPtr secret)
+
+{
+char *uuid = NULL;
+int ret = -1;
+
+uuid = virXPathString(string(./auth/secret/@uuid), ctxt);
+secret-usage = virXPathString(string(./auth/secret/@usage), ctxt);
+if (uuid == NULL  secret-usage == NULL) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(missing auth secret uuid or usage attribute));
+return -1;
+}
+
+if (uuid != NULL) {
+if (secret-usage != NULL) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(either auth secret uuid or usage expected));
+goto cleanup;
+}
+if (virUUIDParse(uuid, secret-uuid)  0) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(invalid auth secret uuid));
+goto cleanup;
+}
+secret-uuidUsable = true;
+} else {
+secret-uuidUsable = false;
+}
+
+ret = 0;
+cleanup:
+VIR_FREE(uuid);
+return ret;
+}
 
 static int
 virStoragePoolDefParseAuthChap(xmlXPathContextPtr ctxt,
@@ -484,9 +521,6 @@ static int
 virStoragePoolDefParseAuthCephx(xmlXPathContextPtr ctxt,
 virStoragePoolAuthCephxPtr auth)
 {
-char *uuid = NULL;
-int ret = -1;
-
 auth-username = virXPathString(string(./auth/@username), ctxt);
 if (auth-username == NULL) {
 virReportError(VIR_ERR_XML_ERROR, %s,
@@ -494,34 +528,10 @@ virStoragePoolDefParseAuthCephx(xmlXPathContextPtr ctxt,
 return -1;
 }
 
-uuid = virXPathString(string(./auth/secret/@uuid), ctxt);
-auth-secret.usage = virXPathString(string(./auth/secret/@usage), ctxt);
-if (uuid == NULL  auth-secret.usage == NULL) {
-virReportError(VIR_ERR_XML_ERROR, %s,
-   _(missing auth secret uuid or usage attribute));
+if (virStoragePoolDefParseAuthSecret(ctxt, auth-secret)  0)
 return -1;
-}
-
-if (uuid != NULL) {
-if (auth-secret.usage != NULL) {
-virReportError(VIR_ERR_XML_ERROR, %s,
-   _(either auth secret uuid or usage expected));
-goto cleanup;
-}
-if (virUUIDParse(uuid, auth-secret.uuid)  0) {
-virReportError(VIR_ERR_XML_ERROR, %s,
-   _(invalid auth secret uuid));
-goto cleanup;
-}
-auth-secret.uuidUsable = true;
-} else {
-auth-secret.uuidUsable = false;
-}
 
-ret = 0;
-cleanup:
-VIR_FREE(uuid);
-return ret;
+return 0;
 }
 
 static int
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 8e739ff..aff1393 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -145,6 +145,14 @@ enum virStoragePoolAuthType {
 VIR_STORAGE_POOL_AUTH_CEPHX,
 };
 
+typedef struct _virStoragePoolAuthSecret virStoragePoolAuthSecret;
+typedef virStoragePoolAuthSecret *virStoragePoolAuthSecretPtr;
+struct _virStoragePoolAuthSecret {
+unsigned char uuid[VIR_UUID_BUFLEN];
+char *usage;
+bool uuidUsable;
+};
+
 typedef struct _virStoragePoolAuthChap virStoragePoolAuthChap;
 typedef virStoragePoolAuthChap *virStoragePoolAuthChapPtr;
 struct _virStoragePoolAuthChap {
@@ -156,11 +164,7 @@ typedef struct _virStoragePoolAuthCephx 
virStoragePoolAuthCephx;
 typedef virStoragePoolAuthCephx *virStoragePoolAuthCephxPtr;
 struct _virStoragePoolAuthCephx {
 char *username;
-struct {
-unsigned char uuid[VIR_UUID_BUFLEN];
-char *usage;
-bool uuidUsable;
-} secret;
+virStoragePoolAuthSecret secret;
 };
 
 /*
-- 
1.8.1.4

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


[libvirt] [PATCH 02/11] storage: Support username for chap type auth

2013-05-28 Thread Osier Yang
To be consistent with what we use in disk auth, and ceph type
storage auth, this supports username. login is still supported
for back-compat reason.
---
 docs/schemas/storagepool.rng   | 12 +++---
 src/conf/storage_conf.c| 27 ++
 .../storagepoolxml2xmlin/pool-iscsi-auth-login.xml | 17 ++
 .../pool-iscsi-auth-username.xml   | 17 ++
 tests/storagepoolxml2xmlin/pool-iscsi-auth.xml | 17 --
 .../pool-iscsi-auth-login.xml  | 20 
 .../pool-iscsi-auth-username.xml   | 20 
 tests/storagepoolxml2xmlout/pool-iscsi-auth.xml| 20 
 .../pool-iscsi-vendor-product.xml  |  2 +-
 tests/storagepoolxml2xmltest.c |  3 ++-
 10 files changed, 109 insertions(+), 46 deletions(-)
 create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-login.xml
 create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-username.xml
 delete mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-login.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-username.xml
 delete mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth.xml

diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 2595e37..ba6c741 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -286,9 +286,15 @@
 valuechap/value
   /attribute
   interleave
-attribute name='login'
-  text/
-/attribute
+choice
+  !-- Legacy, but still supported to keep back-compat --
+  attribute name='login'
+text/
+  /attribute
+  attribute name='username'
+text/
+  /attribute
+/choice
 attribute name='passwd'
   text/
 /attribute
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index ed9effd..c0bf084 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -444,13 +444,32 @@ static int
 virStoragePoolDefParseAuthChap(xmlXPathContextPtr ctxt,
virStoragePoolAuthChapPtr auth)
 {
-auth-login = virXPathString(string(./auth/@login), ctxt);
-if (auth-login == NULL) {
+char *login = NULL;
+char *username = NULL;
+
+login = virXPathString(string(./auth/@login), ctxt);
+username = virXPathString(string(./auth/@username), ctxt);
+
+if (!login  !username) {
 virReportError(VIR_ERR_XML_ERROR, %s,
-   _(missing auth login attribute));
+   _(missing auth login or username attribute));
 return -1;
 }
 
+if (login  username) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _('login' is legacy name of 'username', they 
+ are exclusive));
+VIR_FREE(login);
+VIR_FREE(username);
+return -1;
+}
+
+if (login)
+auth-login = login;
+else if (username)
+auth-login = username;
+
 auth-passwd = virXPathString(string(./auth/@passwd), ctxt);
 if (auth-passwd == NULL) {
 virReportError(VIR_ERR_XML_ERROR, %s,
@@ -1101,7 +1120,7 @@ virStoragePoolSourceFormat(virBufferPtr buf,
 }
 
 if (src-authType == VIR_STORAGE_POOL_AUTH_CHAP)
-virBufferAsprintf(buf,auth type='chap' login='%s' 
passwd='%s'/\n,
+virBufferAsprintf(buf,auth type='chap' username='%s' 
passwd='%s'/\n,
   src-auth.chap.login,
   src-auth.chap.passwd);
 
diff --git a/tests/storagepoolxml2xmlin/pool-iscsi-auth-login.xml 
b/tests/storagepoolxml2xmlin/pool-iscsi-auth-login.xml
new file mode 100644
index 000..f7d4d52
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-iscsi-auth-login.xml
@@ -0,0 +1,17 @@
+pool type='iscsi'
+  namevirtimages/name
+  uuide9392370-2917-565e-692b-d057f46512d6/uuid
+  source
+host name=iscsi.example.com/
+device path=demo-target/
+auth type='chap' login='foobar' passwd='frobbar'/
+  /source
+  target
+path/dev/disk/by-path/path
+permissions
+  mode0700/mode
+  owner0/owner
+  group0/group
+/permissions
+  /target
+/pool
diff --git a/tests/storagepoolxml2xmlin/pool-iscsi-auth-username.xml 
b/tests/storagepoolxml2xmlin/pool-iscsi-auth-username.xml
new file mode 100644
index 000..b8e4d37
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-iscsi-auth-username.xml
@@ -0,0 +1,17 @@
+pool type='iscsi'
+  namevirtimages/name
+  uuide9392370-2917-565e-692b-d057f46512d6/uuid
+  source
+host name=iscsi.example.com/
+device path=demo-target/
+auth type='chap' username='foobar' passwd='frobbar'/
+  /source
+  target
+path/dev/disk/by-path/path
+permissions
+ 

[libvirt] [PATCH 06/11] storage: Support chap authentication for iscsi pool

2013-05-28 Thread Osier Yang
Though the XML for chap authentication with plain password
was introduced long ago, the function was never implemented.
This patch completes it.

There are two types of CHAP configurations supported for iSCSI
authentication:
  * Initiator Authentication
Forward, one-way; The initiator is authenticated by the target.

  * Target Authentication
Reverse, Bi-directional, mutual, two-way; The target is authenticated
by the initiator; This method also requires Initiator Authentication

This only supports the Initiator Authentication. (I don't have any
enterprise iSCSI env for testing, only have a iSCSI target setup with
tgtd, which doesn't support Target Authentication).

Discovery authentication is not supported by tgt yet too. So this only
setup the session authentication by executing 3 iscsiadm commands, E.g:

% iscsiadm -m node --target iqn.2013-05.test:iscsi.foo --name \
  node.session.auth.authmethod -v CHAP --op update

% iscsiadm -m node --target iqn.2013-05.test:iscsi.foo --name \
  node.session.auth.username -v Jim --op update

% iscsiadm -m node --target iqn.2013-05.test:iscsi.foo --name \
  node.session.auth.password -v Jimsecret --op update
---
 src/storage/storage_backend_iscsi.c | 68 +
 1 file changed, 68 insertions(+)

diff --git a/src/storage/storage_backend_iscsi.c 
b/src/storage/storage_backend_iscsi.c
index ad38ab2..6a17b5a 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -713,6 +713,68 @@ virStorageBackendISCSICheckPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 return ret;
 }
 
+static int
+virStorageBackendISCSINodeUpdate(const char *portal,
+ const char *target,
+ const char *name,
+ const char *value)
+{
+ virCommandPtr cmd = NULL;
+ int status;
+ int ret = -1;
+
+ cmd = virCommandNewArgList(ISCSIADM,
+--mode, node,
+--portal, portal,
+--target, target,
+--op, update,
+--name, name,
+--value, value,
+NULL);
+
+if (virCommandRun(cmd, status)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Failed to update '%s' of node mode for target '%s'),
+   name, target);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+virCommandFree(cmd);
+return ret;
+}
+
+static int
+virStorageBackendISCSISetAuth(virStoragePoolDefPtr def,
+  const char *portal,
+  const char *target)
+{
+if (def-source.authType == VIR_STORAGE_POOL_AUTH_NONE)
+return 0;
+
+if (def-source.authType != VIR_STORAGE_POOL_AUTH_CHAP) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(iscsi pool only supports 'chap' auth type));
+return -1;
+}
+
+if (virStorageBackendISCSINodeUpdate(portal,
+ target,
+ node.session.auth.authmethod,
+ CHAP)  0 ||
+virStorageBackendISCSINodeUpdate(portal,
+ target,
+ node.session.auth.username,
+ def-source.auth.chap.login)  0 ||
+virStorageBackendISCSINodeUpdate(portal,
+ target,
+ node.session.auth.password,
+ def-source.auth.chap.u.passwd)  0)
+return -1;
+
+return 0;
+}
 
 static int
 virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -745,6 +807,7 @@ virStorageBackendISCSIStartPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 if ((session = virStorageBackendISCSISession(pool, 1)) == NULL) {
 if ((portal = virStorageBackendISCSIPortal(pool-def-source)) == 
NULL)
 goto cleanup;
+
 /*
  * iscsiadm doesn't let you login to a target, unless you've
  * first issued a 'sendtargets' command to the portal :-(
@@ -754,6 +817,11 @@ virStorageBackendISCSIStartPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
   NULL, NULL)  0)
 goto cleanup;
 
+if (virStorageBackendISCSISetAuth(pool-def,
+  portal,
+  pool-def-source.devices[0].path)  
0)
+goto cleanup;
+
 if (virStorageBackendISCSIConnection(portal,
  pool-def-source.initiator.iqn,
  pool-def-source.devices[0].path,
-- 
1.8.1.4

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

[libvirt] [PATCH 09/11] storage: Use the internal API to get the secret value instead

2013-05-28 Thread Osier Yang
Without the flag VIR_SECRET_GET_VALUE_INTERNAL_CALL, there is no
way to get the value of private secret. And error out if the
secret value is not found.
---
 src/storage/storage_backend_rbd.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 953a8ee..d66d3f9 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -23,6 +23,7 @@
 
 #include config.h
 
+#include datatypes.h
 #include virerror.h
 #include storage_backend_rbd.h
 #include storage_conf.h
@@ -88,7 +89,17 @@ static int 
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr *ptr,
 goto cleanup;
 }
 
-secret_value = virSecretGetValue(secret, secret_value_size, 0);
+secret_value = conn-secretDriver-secretGetValue(secret, 
secret_value_size, 0,
+  
VIR_SECRET_GET_VALUE_INTERNAL_CALL);
+
+if (!secret_value) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(could not get the value of the secret 
+ for username %s),
+   pool-def-source.auth.cephx.username);
+goto cleanup;
+}
+
 base64_encode_alloc((char *)secret_value,
 secret_value_size, rados_key);
 memset(secret_value, 0, secret_value_size);
-- 
1.8.1.4

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


[libvirt] [PATCH 10/11] storage: Improve the pool auth type parsing and formating

2013-05-28 Thread Osier Yang
Use the helpers Type{From,To}String instead.
---
 src/conf/storage_conf.c | 17 ++---
 src/conf/storage_conf.h |  3 +++
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index a876332..bc8ca33 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -94,6 +94,10 @@ VIR_ENUM_IMPL(virStoragePoolSourceAdapterType,
   VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_LAST,
   default, scsi_host, fc_host)
 
+VIR_ENUM_IMPL(virStoragePoolAuthType,
+  VIR_STORAGE_POOL_AUTH_LAST,
+  none, chap, ceph)
+
 typedef const char *(*virStorageVolFormatToString)(int format);
 typedef int (*virStorageVolFormatFromString)(const char *format);
 
@@ -709,11 +713,8 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
 if (authType == NULL) {
 source-authType = VIR_STORAGE_POOL_AUTH_NONE;
 } else {
-if (STREQ(authType, chap)) {
-source-authType = VIR_STORAGE_POOL_AUTH_CHAP;
-} else if (STREQ(authType, ceph)) {
-source-authType = VIR_STORAGE_POOL_AUTH_CEPHX;
-} else {
+if ((source-authType =
+ virStoragePoolAuthTypeTypeFromString(authType))  0) {
 virReportError(VIR_ERR_XML_ERROR,
_(unknown auth type '%s'),
authType);
@@ -1162,7 +1163,8 @@ virStoragePoolSourceFormat(virBufferPtr buf,
 }
 
 if (src-authType == VIR_STORAGE_POOL_AUTH_CHAP) {
-virBufferAsprintf(buf,auth type='chap' username='%s',
+virBufferAsprintf(buf,auth type='%s' username='%s',
+  virStoragePoolAuthTypeTypeToString(src-authType),
   src-auth.chap.login);
 if (src-auth.chap.type == VIR_STORAGE_POOL_AUTH_CHAP_PLAIN_PASSWORD) {
 virBufferAsprintf(buf,  passwd='%s'/\n,
@@ -1175,7 +1177,8 @@ virStoragePoolSourceFormat(virBufferPtr buf,
 }
 
 if (src-authType == VIR_STORAGE_POOL_AUTH_CEPHX) {
-virBufferAsprintf(buf,auth type='ceph' username='%s'\n,
+virBufferAsprintf(buf,auth type='%s' username='%s'\n,
+  virStoragePoolAuthTypeTypeToString(src-authType),
   src-auth.cephx.username);
 virStoragePoolAuthDefFormat(buf, src-auth.cephx.secret);
 virBufferAddLit(buf,/auth\n);
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index b52cdc4..06cd5c0 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -143,7 +143,10 @@ enum virStoragePoolAuthType {
 VIR_STORAGE_POOL_AUTH_NONE,
 VIR_STORAGE_POOL_AUTH_CHAP,
 VIR_STORAGE_POOL_AUTH_CEPHX,
+
+VIR_STORAGE_POOL_AUTH_LAST,
 };
+VIR_ENUM_DECL(virStoragePoolAuthType)
 
 typedef struct _virStoragePoolAuthSecret virStoragePoolAuthSecret;
 typedef virStoragePoolAuthSecret *virStoragePoolAuthSecretPtr;
-- 
1.8.1.4

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


[libvirt] [PATCH] util/vshstring: fix the vir_strdup when src is NULL.

2013-05-28 Thread yangdongsheng
When src is NULL, vir_strdup will return 0 directly.
This patch will set dest to NULL before vir_strdup return.

Signed-off-by: yangdongsheng yangds.f...@cn.fujitsu.com
---
 src/util/virstring.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/util/virstring.c b/src/util/virstring.c
index b244e6c..25b5d81 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -540,8 +540,10 @@ virStrdup(char **dest,
   const char *funcname,
   size_t linenr)
 {
-if (!src)
+if (!src) {
+*dest = NULL;
 return 0;
+}
 if (!(*dest = strdup(src))) {
 if (report)
 virReportOOMErrorFull(domcode, filename, funcname, linenr);
-- 
1.7.10.1

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


Re: [libvirt] Entering freeze for libvirt-1.0.6

2013-05-28 Thread Viktor Mihajlovski

On 05/27/2013 03:44 PM, Daniel Veillard wrote:

   As planned, I just tagged the rc1 release in git and pushed
tarball and rpms to the FTP repository:
   ftp://libvirt.org/

So focuse should be given to bug fixes this week, and if everything
goes well I will make the release on Monday 3, a week from now.
I gave a bit of testing to the rc1 rpms, seems to work okay for KVM on
Fedora but further testing would  be a good idea, on other
distro/platforms and also other hypervisor, especially with VirtualBox
as the driver was mdified to now rely on the daemon for support of
that hypervisor.

   thanks in advance,

Daniel


Basic tests on s390 show no problems so far.

--

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research  Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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


Re: [libvirt] [sandbox][PATCH] Fix nits in virt-sandbox-service when raise ValueError

2013-05-28 Thread Alex Jia

On 05/28/2013 02:01 PM, Wayne Sun wrote:

Put error msg in list when raise ValueError.

This fix is for bug:
[virt-sandbox-service] execute command with unsupported URI error msg is not 
right
https://bugzilla.redhat.com/show_bug.cgi?id=967705

Signed-off-by: Wayne Sung...@redhat.com
---
  bin/virt-sandbox-service |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 4496b29..1db3c09 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -878,7 +878,7 @@ def sandbox_list(args):
  def sandbox_reload(args):
  config = read_config(args.name)
  if isinstance(config, gi.repository.LibvirtSandbox.ConfigServiceGeneric):
-raise ValueError(_(Generic Containers do not support reload))
+raise ValueError([_(Generic Containers do not support reload)])
  container = SystemdContainer(uri = args.uri, config = config)
  container.reload(args.unitfiles)

@@ -931,7 +931,7 @@ def fullpath(cmd):

  def execute(args):
  if args.uri != lxc:///:
-raise ValueError(_(Can only execute commands inside of linux 
containers.))
+raise ValueError([_(Can only execute commands inside of linux 
containers.)])

  myexec = [ virsh, -c, args.uri, lxc-enter-namespace ]
  #myexec = [ virt-sandbox-service-util, execute ]


ACK.

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


[libvirt] [PATCH v3 3/3] virsh: supports guest panicked

2013-05-28 Thread Chen Fan
Add 'virsh domstate --reason' command is effective when domain crashed.

---
 tools/virsh-domain-monitor.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 5ed89d1..4bddefe 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -192,6 +192,8 @@ vshDomainStateReasonToString(int state, int reason)
 return N_(save canceled);
 case VIR_DOMAIN_RUNNING_WAKEUP:
 return N_(event wakeup);
+case VIR_DOMAIN_RUNNING_CRASHED:
+return N_(from crashed);
 case VIR_DOMAIN_RUNNING_UNKNOWN:
 case VIR_DOMAIN_RUNNING_LAST:
 ;
@@ -226,6 +228,8 @@ vshDomainStateReasonToString(int state, int reason)
 return N_(shutting down);
 case VIR_DOMAIN_PAUSED_SNAPSHOT:
 return N_(creating snapshot);
+case VIR_DOMAIN_PAUSED_GUEST_PANICKED:
+return N_(guest panicked);
 case VIR_DOMAIN_PAUSED_UNKNOWN:
 case VIR_DOMAIN_PAUSED_LAST:
 ;
@@ -236,6 +240,8 @@ vshDomainStateReasonToString(int state, int reason)
 switch ((virDomainShutdownReason) reason) {
 case VIR_DOMAIN_SHUTDOWN_USER:
 return N_(user);
+case VIR_DOMAIN_SHUTDOWN_CRASHED:
+return N_(crashed);
 case VIR_DOMAIN_SHUTDOWN_UNKNOWN:
 case VIR_DOMAIN_SHUTDOWN_LAST:
 ;
@@ -266,6 +272,8 @@ vshDomainStateReasonToString(int state, int reason)
 
 case VIR_DOMAIN_CRASHED:
 switch ((virDomainCrashedReason) reason) {
+case VIR_DOMAIN_CRASHED_PANICKED:
+return N_(panicked);
 case VIR_DOMAIN_CRASHED_UNKNOWN:
 case VIR_DOMAIN_CRASHED_LAST:
 ;
-- 
1.8.1.4

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


[libvirt] [PATCH v3 2/3] qemu: Supports guest panicked

2013-05-28 Thread Chen Fan
Add monitor callback API domainGUESTPanicked, that
implements the 'on_crash' behavior in the XML when domain crashed.
---
 src/qemu/qemu_monitor.c  |  14 +-
 src/qemu/qemu_monitor.h  |   5 +++
 src/qemu/qemu_monitor_json.c |   7 +++
 src/qemu/qemu_process.c  | 103 ++-
 4 files changed, 127 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 4e35f79..e0cd62c 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -113,7 +113,7 @@ VIR_ENUM_IMPL(qemuMonitorVMStatus,
   QEMU_MONITOR_VM_STATUS_LAST,
   debug, inmigrate, internal-error, io-error, paused,
   postmigrate, prelaunch, finish-migrate, restore-vm,
-  running, save-vm, shutdown, watchdog)
+  running, save-vm, shutdown, watchdog, guest-panicked)
 
 typedef enum {
 QEMU_MONITOR_BLOCK_IO_STATUS_OK,
@@ -1032,6 +1032,15 @@ int qemuMonitorEmitResume(qemuMonitorPtr mon)
 }
 
 
+int qemuMonitorEmitGUESTPanicked(qemuMonitorPtr mon)
+{
+int ret = -1;
+VIR_DEBUG(mon=%p, mon);
+QEMU_MONITOR_CALLBACK(mon, ret, domainGUESTPanicked, mon-vm);
+return ret;
+}
+
+
 int qemuMonitorEmitRTCChange(qemuMonitorPtr mon, long long offset)
 {
 int ret = -1;
@@ -3185,6 +3194,9 @@ int qemuMonitorVMStatusToPausedReason(const char *status)
 case QEMU_MONITOR_VM_STATUS_WATCHDOG:
 return VIR_DOMAIN_PAUSED_WATCHDOG;
 
+case QEMU_MONITOR_VM_STATUS_GUEST_PANICKED:
+return VIR_DOMAIN_PAUSED_GUEST_PANICKED;
+
 /* unreachable from this point on */
 case QEMU_MONITOR_VM_STATUS_LAST:
 ;
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index a607712..543050c 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -140,6 +140,8 @@ struct _qemuMonitorCallbacks {
unsigned long long actual);
 int (*domainPMSuspendDisk)(qemuMonitorPtr mon,
virDomainObjPtr vm);
+int (*domainGUESTPanicked)(qemuMonitorPtr mon,
+   virDomainObjPtr vm);
 };
 
 char *qemuMonitorEscapeArg(const char *in);
@@ -220,6 +222,8 @@ int qemuMonitorEmitBlockJob(qemuMonitorPtr mon,
 int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon,
  unsigned long long actual);
 int qemuMonitorEmitPMSuspendDisk(qemuMonitorPtr mon);
+int qemuMonitorEmitGUESTPanicked(qemuMonitorPtr mon);
+
 
 int qemuMonitorStartCPUs(qemuMonitorPtr mon,
  virConnectPtr conn);
@@ -239,6 +243,7 @@ typedef enum {
 QEMU_MONITOR_VM_STATUS_SAVE_VM,
 QEMU_MONITOR_VM_STATUS_SHUTDOWN,
 QEMU_MONITOR_VM_STATUS_WATCHDOG,
+QEMU_MONITOR_VM_STATUS_GUEST_PANICKED,
 
 QEMU_MONITOR_VM_STATUS_LAST
 } qemuMonitorVMStatus;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2b73884..b6efa52 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -74,6 +74,7 @@ static void 
qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon, virJSONVal
 static void qemuMonitorJSONHandleBlockJobReady(qemuMonitorPtr mon, 
virJSONValuePtr data);
 static void qemuMonitorJSONHandleBalloonChange(qemuMonitorPtr mon, 
virJSONValuePtr data);
 static void qemuMonitorJSONHandlePMSuspendDisk(qemuMonitorPtr mon, 
virJSONValuePtr data);
+static void qemuMonitorJSONHandleGUESTPanicked(qemuMonitorPtr mon, 
virJSONValuePtr data);
 
 typedef struct {
 const char *type;
@@ -87,6 +88,7 @@ static qemuEventHandler eventHandlers[] = {
 { BLOCK_JOB_COMPLETED, qemuMonitorJSONHandleBlockJobCompleted, },
 { BLOCK_JOB_READY, qemuMonitorJSONHandleBlockJobReady, },
 { DEVICE_TRAY_MOVED, qemuMonitorJSONHandleTrayChange, },
+{ GUEST_PANICKED, qemuMonitorJSONHandleGUESTPanicked, },
 { POWERDOWN, qemuMonitorJSONHandlePowerdown, },
 { RESET, qemuMonitorJSONHandleReset, },
 { RESUME, qemuMonitorJSONHandleResume, },
@@ -593,6 +595,11 @@ static void qemuMonitorJSONHandleResume(qemuMonitorPtr 
mon, virJSONValuePtr data
 qemuMonitorEmitResume(mon);
 }
 
+static void qemuMonitorJSONHandleGUESTPanicked(qemuMonitorPtr mon, 
virJSONValuePtr data ATTRIBUTE_UNUSED)
+{
+qemuMonitorEmitGUESTPanicked(mon);
+}
+
 static void qemuMonitorJSONHandleRTCChange(qemuMonitorPtr mon, virJSONValuePtr 
data)
 {
 long long offset = 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d4fd4fb..29c8c4b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -548,6 +548,7 @@ qemuProcessFakeReboot(void *opaque)
 qemuDomainObjPrivatePtr priv = vm-privateData;
 virDomainEventPtr event = NULL;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+virDomainRunningReason reason = VIR_DOMAIN_RUNNING_BOOTED;
 int ret = -1;
 VIR_DEBUG(vm=%p, vm);
 virObjectLock(vm);
@@ -573,8 +574,11 @@ qemuProcessFakeReboot(void *opaque)
 goto endjob;
 }
 
+if 

[libvirt] [PATCH v3 0/3] libvirt supports Guest panicked

2013-05-28 Thread Chen Fan
This patchs implement the 'on_crash' behavior in the XML
 
example XML:
on_crashdestroy/on_crash

Changes:
  v2-v3: 1. split into 3 patches

  v1-v2: 1. fix the incorrect domain state: paused - crashed, when crash
the guest while libvirt isn't running, then restart libvirtd.

Chen Fan (3):
  libvirt: Define domain crash event types
  qemu: Supports guest panicked
  virsh: supports guest panicked

 examples/domain-events/events-c/event-test.c |  10 +++
 include/libvirt/libvirt.h.in |  16 +
 src/conf/domain_conf.c   |  12 ++--
 src/qemu/qemu_monitor.c  |  14 +++-
 src/qemu/qemu_monitor.h  |   5 ++
 src/qemu/qemu_monitor_json.c |   7 ++
 src/qemu/qemu_process.c  | 103 ++-
 tools/virsh-domain-monitor.c |   8 +++
 8 files changed, 169 insertions(+), 6 deletions(-)

-- 
1.8.1.4

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


[libvirt] [PATCH v3 1/3] libvirt: Define domain crash event types

2013-05-28 Thread Chen Fan
This patch introduces domain crashed types and crashed reasons which
will be used while guest panicked.

---
 examples/domain-events/events-c/event-test.c | 10 ++
 include/libvirt/libvirt.h.in | 16 
 src/conf/domain_conf.c   | 12 
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/examples/domain-events/events-c/event-test.c 
b/examples/domain-events/events-c/event-test.c
index eeff50f..1b425fb 100644
--- a/examples/domain-events/events-c/event-test.c
+++ b/examples/domain-events/events-c/event-test.c
@@ -93,6 +93,9 @@ const char *eventToString(int event) {
 case VIR_DOMAIN_EVENT_PMSUSPENDED:
 ret = PMSuspended;
 break;
+case VIR_DOMAIN_EVENT_CRASHED:
+ret = Crashed;
+break;
 }
 return ret;
 }
@@ -209,6 +212,13 @@ static const char *eventDetailToString(int event, int 
detail) {
 break;
 }
 break;
+case VIR_DOMAIN_EVENT_CRASHED:
+   switch ((virDomainEventCrashedDetailType) detail) {
+   case VIR_DOMAIN_EVENT_CRASHED_PANICKED:
+   ret = Panicked;
+   break;
+   }
+   break;
 }
 return ret;
 }
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 1804c93..56c6c5c 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -155,6 +155,7 @@ typedef enum {
 VIR_DOMAIN_RUNNING_SAVE_CANCELED = 7,   /* returned from failed save 
process */
 VIR_DOMAIN_RUNNING_WAKEUP = 8,  /* returned from pmsuspended due to
wakeup event */
+VIR_DOMAIN_RUNNING_CRASHED = 9,  /* resumed from crashed */
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_RUNNING_LAST
@@ -180,6 +181,7 @@ typedef enum {
 VIR_DOMAIN_PAUSED_FROM_SNAPSHOT = 7, /* paused after restoring from 
snapshot */
 VIR_DOMAIN_PAUSED_SHUTTING_DOWN = 8, /* paused during shutdown process */
 VIR_DOMAIN_PAUSED_SNAPSHOT = 9,  /* paused while creating a snapshot */
+VIR_DOMAIN_PAUSED_GUEST_PANICKED = 10, /* paused due to a guest panicked 
event */
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_PAUSED_LAST
@@ -189,6 +191,7 @@ typedef enum {
 typedef enum {
 VIR_DOMAIN_SHUTDOWN_UNKNOWN = 0,/* the reason is unknown */
 VIR_DOMAIN_SHUTDOWN_USER = 1,   /* shutting down on user request */
+VIR_DOMAIN_SHUTDOWN_CRASHED = 2,/* domain crashed */
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_SHUTDOWN_LAST
@@ -212,6 +215,7 @@ typedef enum {
 
 typedef enum {
 VIR_DOMAIN_CRASHED_UNKNOWN = 0, /* crashed for unknown reason */
+VIR_DOMAIN_CRASHED_PANICKED = 1, /* domain panicked */
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_CRASHED_LAST
@@ -3319,6 +3323,7 @@ typedef enum {
 VIR_DOMAIN_EVENT_STOPPED = 5,
 VIR_DOMAIN_EVENT_SHUTDOWN = 6,
 VIR_DOMAIN_EVENT_PMSUSPENDED = 7,
+VIR_DOMAIN_EVENT_CRASHED = 8,
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_EVENT_LAST
@@ -3450,6 +3455,17 @@ typedef enum {
 #endif
 } virDomainEventPMSuspendedDetailType;
 
+/*
+ * Details about the 'crashed' lifecycle event
+ */
+typedef enum {
+VIR_DOMAIN_EVENT_CRASHED_PANICKED = 0, /* Guest was panicked */
+
+#ifdef VIR_ENUM_SENTINELS
+VIR_DOMAIN_EVENT_CRASHED_LAST
+#endif
+} virDomainEventCrashedDetailType;
+
 /**
  * virConnectDomainEventCallback:
  * @conn: virConnect connection
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a9656af..f577fd4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -642,7 +642,8 @@ VIR_ENUM_IMPL(virDomainRunningReason, 
VIR_DOMAIN_RUNNING_LAST,
   unpaused,
   migration canceled,
   save canceled,
-  wakeup)
+  wakeup,
+  from crashed)
 
 VIR_ENUM_IMPL(virDomainBlockedReason, VIR_DOMAIN_BLOCKED_LAST,
   unknown)
@@ -657,11 +658,13 @@ VIR_ENUM_IMPL(virDomainPausedReason, 
VIR_DOMAIN_PAUSED_LAST,
   watchdog,
   from snapshot,
   shutdown,
-  snapshot)
+  snapshot,
+  guest panicked)
 
 VIR_ENUM_IMPL(virDomainShutdownReason, VIR_DOMAIN_SHUTDOWN_LAST,
   unknown,
-  user)
+  user,
+  crashed)
 
 VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST,
   unknown,
@@ -674,7 +677,8 @@ VIR_ENUM_IMPL(virDomainShutoffReason, 
VIR_DOMAIN_SHUTOFF_LAST,
   from snapshot)
 
 VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST,
-  unknown)
+  unknown,
+  panicked)
 
 VIR_ENUM_IMPL(virDomainPMSuspendedReason, VIR_DOMAIN_PMSUSPENDED_LAST,
   unknown)
-- 
1.8.1.4

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


Re: [libvirt] [PATCH] util/vshstring: fix the vir_strdup when src is NULL.

2013-05-28 Thread Yang Dongsheng

This bug will cause the dest get a unexpected value.

Example:
[root@yds-pc libvirt]# virsh
Welcome to virsh, the virtualization interactive terminal.

Type: 'help' for help with commands
'quit' to quit

virsh # connect
error: Failed to connect to the hypervisor
error: internal error Unable to parse URI �N�*C

Please someone give me a comment. Thanx~

On 05/28/2013 03:13 PM, yangdongsheng wrote:

When src is NULL, vir_strdup will return 0 directly.
This patch will set dest to NULL before vir_strdup return.

Signed-off-by: yangdongshengyangds.f...@cn.fujitsu.com
---
  src/util/virstring.c |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/util/virstring.c b/src/util/virstring.c
index b244e6c..25b5d81 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -540,8 +540,10 @@ virStrdup(char **dest,
const char *funcname,
size_t linenr)
  {
-if (!src)
+if (!src) {
+*dest = NULL;
  return 0;
+}
  if (!(*dest = strdup(src))) {
  if (report)
  virReportOOMErrorFull(domcode, filename, funcname, linenr);



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

[libvirt] [PATCH] qemu: match vcpu topology to maximum vcpu numbers.

2013-05-28 Thread Guannan Ren
For qemu, if the -smp N or the value of maxcpus is given, it define
the number of vcpu of guest whenever the vcpu topology is defined or
not. But if the -smp N and maxcpus are missing, the topology can
compute and define vcpus for guest automatically by math:

vcpu number = sockets*cores*threads

For libvirt, as vcpu is always mandatory, so we can ask topology
to match maximum vcpu numbers.
---
 src/conf/domain_conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a9656af..ffdc6da 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11815,10 +11815,10 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 
 if (def-cpu-sockets 
-def-maxvcpus 
+def-maxvcpus !=
 def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
 virReportError(VIR_ERR_XML_DETAIL, %s,
-   _(Maximum CPUs greater than topology limit));
+   _(Topology limit does not match maximum CPUs));
 goto error;
 }
 
-- 
1.8.1.4

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


[libvirt] [PATCH 3/3] virsh-domain: Add --live, --config, --current logic to cmdAttachInterface

2013-05-28 Thread Peter Krempa
Use the approach established in commit
69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too.
---
 tools/virsh-domain.c | 59 +++-
 tools/virsh.pod  | 34 +-
 2 files changed, 60 insertions(+), 33 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4de5dd5..96f6765 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -724,14 +724,6 @@ static const vshCmdOptDef opts_attach_interface[] = {
  .type = VSH_OT_DATA,
  .help = N_(model type)
 },
-{.name = persistent,
- .type = VSH_OT_ALIAS,
- .help = config
-},
-{.name = config,
- .type = VSH_OT_BOOL,
- .help = N_(affect next boot)
-},
 {.name = inbound,
  .type = VSH_OT_DATA,
  .help = N_(control domain's incoming traffics)
@@ -740,6 +732,22 @@ static const vshCmdOptDef opts_attach_interface[] = {
  .type = VSH_OT_DATA,
  .help = N_(control domain's outgoing traffics)
 },
+{.name = persistent,
+ .type = VSH_OT_BOOL,
+ .help = N_(make live change persistent)
+},
+{.name = config,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect next boot)
+},
+{.name = live,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect running domain)
+},
+{.name = current,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect current domain)
+},
 {.name = NULL}
 };

@@ -789,12 +797,30 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
 int typ;
 int ret;
 bool functionReturn = false;
-unsigned int flags;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 char *xml;
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+bool current = vshCommandOptBool(cmd, current);
+bool config = vshCommandOptBool(cmd, config);
+bool live = vshCommandOptBool(cmd, live);
+bool persistent = vshCommandOptBool(cmd, persistent);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+if (config || persistent)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_AFFECT_LIVE;

 if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
-goto cleanup;
+return false;
+
+if (persistent 
+virDomainIsActive(dom) == 1)
+flags |= VIR_DOMAIN_AFFECT_LIVE;

 if (vshCommandOptStringReq(ctl, cmd, type, type)  0 ||
 vshCommandOptStringReq(ctl, cmd, source, source)  0 ||
@@ -887,14 +913,10 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)

 xml = virBufferContentAndReset(buf);

-if (vshCommandOptBool(cmd, config)) {
-flags = VIR_DOMAIN_AFFECT_CONFIG;
-if (virDomainIsActive(dom) == 1)
-flags |= VIR_DOMAIN_AFFECT_LIVE;
+if (flags)
 ret = virDomainAttachDeviceFlags(dom, xml, flags);
-} else {
+else
 ret = virDomainAttachDevice(dom, xml);
-}

 VIR_FREE(xml);

@@ -905,9 +927,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
 functionReturn = true;
 }

- cleanup:
-if (dom)
-virDomainFree(dom);
+cleanup:
+virDomainFree(dom);
 virBufferFreeAndReset(buf);
 return functionReturn;
 }
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 9ae96d1..0fd546d 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1885,25 +1885,31 @@ For compatibility purposes, I--persistent behaves 
like I--config for
 an offline domain, and like I--live I--config for a running domain.

 =item Battach-interface Idomain Itype Isource
+[[[I--live] [I--config] | [I--current]] | [I--persistent]]
 [I--target target] [I--mac mac] [I--script script] [I--model model]
 [I--config] [I--inbound average,peak,burst] [I--outbound 
average,peak,burst]

-Attach a new network interface to the domain.
-Itype can be either Inetwork to indicate a physical network device or
-Ibridge to indicate a bridge to a device.
-Isource indicates the source device.
-Itarget allows to indicate the target device in the guest. Names starting
-with 'vnet' are considered as auto-generated an hence blanked out.
-Imac allows to specify the MAC address of the network interface.
-Iscript allows to specify a path to a script handling a bridge instead of
-the default one.
-Imodel allows to specify the model type.
-I--config indicates the changes will affect the next boot of the domain,
-for compatibility purposes, I--persistent is alias of I--config.
-Iinbound and Ioutbound control the bandwidth of the interface. Ipeak
-and Iburst are optional, so average,peak, average,,burst and
+Attach a new network interface to the domain.  Itype can be either Inetwork
+to indicate a physical network device or Ibridge to indicate a bridge to a
+device.  Isource indicates the source device.  Itarget allows to indicate
+the target device in the guest. Names starting with 'vnet' are considered as
+auto-generated an hence blanked out.  Imac 

[libvirt] [PATCH 0/3] virsh: Add --live, --config, --current to attach-* commands too

2013-05-28 Thread Peter Krempa
In commit 69ce3ffa8d431e9810607c6e00b7cfcc481b491d a pattern was established to
upgrade hotplug commands in virsh. The series containing the patch forgot
to upgrade the attach-* commands too.

Peter Krempa (3):
  virsh-domain: Add --live, --config, --current logic to cmdAttachDevice
  virsh-domain: Add --live, --config, --current logic to cmdAttachDisk
  virsh-domain: Add --live, --config, --current logic to
cmdAttachInterface

 tools/virsh-domain.c | 153 ---
 tools/virsh.pod  |  63 ++---
 2 files changed, 153 insertions(+), 63 deletions(-)

-- 
1.8.2.1

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


[libvirt] [PATCH 2/3] virsh-domain: Add --live, --config, --current logic to cmdAttachDisk

2013-05-28 Thread Peter Krempa
Use the approach established in commit
69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too.
---
 tools/virsh-domain.c | 53 
 tools/virsh.pod  | 13 +++--
 2 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b965c11..4de5dd5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -299,14 +299,6 @@ static const vshCmdOptDef opts_attach_disk[] = {
  .type = VSH_OT_STRING,
  .help = N_(mode of device reading and writing)
 },
-{.name = persistent,
- .type = VSH_OT_ALIAS,
- .help = config
-},
-{.name = config,
- .type = VSH_OT_BOOL,
- .help = N_(affect next boot)
-},
 {.name = sourcetype,
  .type = VSH_OT_STRING,
  .help = N_(type of source (block|file))
@@ -335,7 +327,22 @@ static const vshCmdOptDef opts_attach_disk[] = {
  .type = VSH_OT_BOOL,
  .help = N_(print XML document rather than attach the disk)
 },
-
+{.name = persistent,
+ .type = VSH_OT_BOOL,
+ .help = N_(make live change persistent)
+},
+{.name = config,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect next boot)
+},
+{.name = live,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect running domain)
+},
+{.name = current,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect current domain)
+},
 {.name = NULL}
 };

@@ -496,15 +503,33 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
 struct DiskAddress diskAddr;
 bool isFile = false, functionReturn = false;
 int ret;
-unsigned int flags;
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
 const char *stype = NULL;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 char *xml = NULL;
 struct stat st;
+bool current = vshCommandOptBool(cmd, current);
+bool config = vshCommandOptBool(cmd, config);
+bool live = vshCommandOptBool(cmd, live);
+bool persistent = vshCommandOptBool(cmd, persistent);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+if (config || persistent)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_AFFECT_LIVE;

 if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
 return false;

+if (persistent 
+virDomainIsActive(dom) == 1)
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+
 if (vshCommandOptStringReq(ctl, cmd, source, source)  0 ||
 vshCommandOptStringReq(ctl, cmd, target, target)  0 ||
 vshCommandOptStringReq(ctl, cmd, driver, driver)  0 ||
@@ -635,14 +660,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
 goto cleanup;
 }

-if (vshCommandOptBool(cmd, config)) {
-flags = VIR_DOMAIN_AFFECT_CONFIG;
-if (virDomainIsActive(dom) == 1)
-flags |= VIR_DOMAIN_AFFECT_LIVE;
+if (flags)
 ret = virDomainAttachDeviceFlags(dom, xml, flags);
-} else {
+else
 ret = virDomainAttachDevice(dom, xml);
-}

 if (ret != 0) {
 vshError(ctl, %s, _(Failed to attach disk));
diff --git a/tools/virsh.pod b/tools/virsh.pod
index a002623..9ae96d1 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1839,6 +1839,7 @@ results as some fields may be autogenerated and thus 
match devices other than
 expected.

 =item Battach-disk Idomain Isource Itarget
+[[[I--live] [I--config] | [I--current]] | [I--persistent]]
 [I--driver driver] [I--subdriver subdriver] [I--cache cache]
 [I--type type] [I--mode mode] [I--config] [I--sourcetype soucetype]
 [I--serial serial] [I--shareable] [I--rawio] [I--address address]
@@ -1859,8 +1860,6 @@ alternative to the disk default, although this use only 
replaces the media
 within the existing virtual cdrom or floppy device; consider using
 Bupdate-device for this usage instead.
 Imode can specify the two specific mode Ireadonly or Ishareable.
-I--config indicates the changes will affect the next boot of the domain,
-for compatibility purposes, I--persistent is alias of I--config.
 Isourcetype can indicate the type of source (block|file)
 Icache can be one of default, none, writethrough, writeback,
 directsync or unsafe.
@@ -1875,6 +1874,16 @@ address.
 If I--print-xml is specified, then the XML of the disk that would be attached
 is printed instead.

+If I--live is specified, affect a running domain.
+If I--config is specified, affect the next startup of a persistent domain.
+If I--current is specified, affect the current domain state.
+Both I--live and I--config flags may be given, but I--current is
+exclusive. When no flag is specified legacy API is used whose behavior depends
+on the hypervisor driver.
+
+For compatibility purposes, I--persistent behaves like I--config for
+an offline domain, and like I--live I--config for a running domain.
+
 =item Battach-interface Idomain Itype Isource
 [I--target target] 

[libvirt] [PATCH 1/3] virsh-domain: Add --live, --config, --current logic to cmdAttachDevice

2013-05-28 Thread Peter Krempa
Use the approach established in commit
69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too.
---
 tools/virsh-domain.c | 41 -
 tools/virsh.pod  | 16 +---
 2 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0402aef..b965c11 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -173,13 +173,21 @@ static const vshCmdOptDef opts_attach_device[] = {
  .help = N_(XML file)
 },
 {.name = persistent,
- .type = VSH_OT_ALIAS,
- .help = config
+ .type = VSH_OT_BOOL,
+ .help = N_(make live change persistent)
 },
 {.name = config,
  .type = VSH_OT_BOOL,
  .help = N_(affect next boot)
 },
+{.name = live,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect running domain)
+},
+{.name = current,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect current domain)
+},
 {.name = NULL}
 };

@@ -191,7 +199,21 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
 char *buffer;
 int rv;
 bool ret = false;
-unsigned int flags;
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+bool current = vshCommandOptBool(cmd, current);
+bool config = vshCommandOptBool(cmd, config);
+bool live = vshCommandOptBool(cmd, live);
+bool persistent = vshCommandOptBool(cmd, persistent);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+if (config || persistent)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_AFFECT_LIVE;

 if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
 return false;
@@ -199,19 +221,20 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptStringReq(ctl, cmd, file, from)  0)
 goto cleanup;

+if (persistent 
+virDomainIsActive(dom) == 1)
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+
 if (virFileReadAll(from, VSH_MAX_XML_FILE, buffer)  0) {
 vshReportError(ctl);
 goto cleanup;
 }

-if (vshCommandOptBool(cmd, config)) {
-flags = VIR_DOMAIN_AFFECT_CONFIG;
-if (virDomainIsActive(dom) == 1)
-   flags |= VIR_DOMAIN_AFFECT_LIVE;
+if (flags)
 rv = virDomainAttachDeviceFlags(dom, buffer, flags);
-} else {
+else
 rv = virDomainAttachDevice(dom, buffer);
-}
+
 VIR_FREE(buffer);

 if (rv  0) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 11984bc..a002623 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1809,7 +1809,8 @@ format of the device sections to get the most accurate 
set of accepted values.

 =over 4

-=item Battach-device Idomain IFILE [I--config]
+=item Battach-device Idomain IFILE
+[[[I--live] [I--config] | [I--current]] | [I--persistent]]

 Attach a device to the domain, using a device definition in an XML
 file using a device definition element such as disk or interface
@@ -1817,13 +1818,22 @@ as the top-level element.  See the documentation at
 Lhttp://libvirt.org/formatdomain.html#elementsDevices to learn about
 libvirt XML format for a device.  If I--config is specified the
 command alters the persistent domain configuration with the device
-attach taking effect the next time libvirt starts the domain.  For
-compatibility purposes, I--persistent is an alias of I--config.
+attach taking effect the next time libvirt starts the domain.
 For cdrom and floppy devices, this command only replaces the media
 within an existing device; consider using Bupdate-device for this
 usage.  For passthrough host devices, see also Bnodedev-detach,
 needed if the device does not use managed mode.

+If I--live is specified, affect a running domain.
+If I--config is specified, affect the next startup of a persistent domain.
+If I--current is specified, affect the current domain state.
+Both I--live and I--config flags may be given, but I--current is
+exclusive. When no flag is specified legacy API is used whose behavior depends
+on the hypervisor driver.
+
+For compatibility purposes, I--persistent behaves like I--config for
+an offline domain, and like I--live I--config for a running domain.
+
 BNote: using of partial device definition XML files may lead to unexpected
 results as some fields may be autogenerated and thus match devices other than
 expected.
-- 
1.8.2.1

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


Re: [libvirt] [PATCH v3 0/3] libvirt supports Guest panicked

2013-05-28 Thread Viktor Mihajlovski

On 05/28/2013 11:00 AM, Chen Fan wrote:

This patchs implement the 'on_crash' behavior in the XML

example XML:
 on_crashdestroy/on_crash

Changes:
   v2-v3: 1. split into 3 patches

   v1-v2: 1. fix the incorrect domain state: paused - crashed, when crash
 the guest while libvirt isn't running, then restart libvirtd.

Chen Fan (3):
   libvirt: Define domain crash event types
   qemu: Supports guest panicked
   virsh: supports guest panicked

  examples/domain-events/events-c/event-test.c |  10 +++
  include/libvirt/libvirt.h.in |  16 +
  src/conf/domain_conf.c   |  12 ++--
  src/qemu/qemu_monitor.c  |  14 +++-
  src/qemu/qemu_monitor.h  |   5 ++
  src/qemu/qemu_monitor_json.c |   7 ++
  src/qemu/qemu_process.c  | 103 ++-
  tools/virsh-domain-monitor.c |   8 +++
  8 files changed, 169 insertions(+), 6 deletions(-)


Applied and tested on s390. Works fine and as expected (by me).
Of course Daniel or Eric should have a final look at it, as this
series extends the domain state model.

--

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research  Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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


Re: [libvirt] [PATCH 08/12] storage_conf: Improve the memory deallocation of virStorageVolDefParseXML

2013-05-28 Thread Ján Tomko
On 05/22/2013 02:05 PM, Osier Yang wrote:
 Changes:
   * Add a new goto label error
   * Free the strings at cleanup
   * Remove the unnecessary frees
 ---
  src/conf/storage_conf.c | 29 ++---
  1 file changed, 14 insertions(+), 15 deletions(-)
 

ACK.

Jan

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


Re: [libvirt] [PATCH 11/12] storage_conf: Improve error messages

2013-05-28 Thread Ján Tomko
On 05/22/2013 02:05 PM, Osier Yang wrote:
 virStoragePoolDefParseSource:
   * Better error message
 
 virStoragePoolObjLoad:
   * Break the line line
 ---
  src/conf/storage_conf.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index 44ecb2a..a62629e 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c
 @@ -531,7 +531,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
  source-name = virXPathString(string(./name), ctxt);
  if (pool_type == VIR_STORAGE_POOL_RBD  source-name == NULL) {
  virReportError(VIR_ERR_XML_ERROR, %s,
 -   _(missing mandatory 'name' field for RBD pool 
 name));
 +   _(element 'name' is mandatory for RBD pool name));

s/pool name/pools/

  goto cleanup;
  }
  
 @@ -1695,7 +1695,8 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
  
  if (!virFileMatchesNameSuffix(file, def-name, .xml)) {
  virReportError(VIR_ERR_XML_ERROR,
 -   _(Storage pool config filename '%s' does not match 
 pool name '%s'),
 +   _(Storage pool config filename '%s' does 
 + not match pool name '%s'),
 path, def-name);
  virStoragePoolDefFree(def);
  return NULL;
 

ACK

Jan

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


Re: [libvirt] [PATCH 07/12] storage_conf: Improve the memory deallocation of pool def parsing

2013-05-28 Thread Ján Tomko
On 05/22/2013 02:05 PM, Osier Yang wrote:
 Changes:
 * Free all the strings at cleanup, instead of freeing them
   in the middle
 * Remove xmlFree
 * s/tmppath/target_path/, to make it more sensible
 * Add new goto label error
 ---
  src/conf/storage_conf.c | 54 
 -
  1 file changed, 26 insertions(+), 28 deletions(-)
 
 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index efe02e8..6f0ed74 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c

 @@ -914,7 +910,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
  virReportError(VIR_ERR_XML_ERROR, %s,
 _('wwnn' and 'wwpn' must be specified for 
 adapter 
   type 'fchost'));
 -goto cleanup;
 +goto error;
  }
  
  if (!virValidateWWN(ret-source.adapter.data.fchost.wwnn) ||

If the WWN validation fails here, you still jump to cleanup instead of error.

ACK if you fix that.

Jan

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


Re: [libvirt] [PATCH 2/2] bridge driver: don't masquerade local subnet broadcast/multicast packets

2013-05-28 Thread Brian J. Murrell
I'm not sure if everyone that needs to see this discussion are
subscribed to RH bugzilla bug #709418 so I will repeat my comments from
comment #12 on that bug here:

On 13-05-27 10:34 PM, Laszlo Ersek wrote:
 Packets sent by guests on virbrN, *or* by dnsmasq on the same, to
 - 255.255.255.255/32 (netmask-independent local network broadcast
   address), or to
 - 224.0.0.0/24 (local subnetwork multicast range)

Multicast is much wider than just 224.0.0.0/24.  Per my patch in
attachment 526549 [details], it's in fact 224.0.0.0/4.  All multicast
needs to be exempt from NAT.

Quoting from Wikipedia article
http://en.wikipedia.org/wiki/Multicast_address:

IPv4 multicast addresses are defined by the leading address bits of
1110, originating from the classful network design of the early Internet
when this group of addresses was designated as Class D. The Classless
Inter-Domain Routing (CIDR) prefix of this group is 224.0.0.0/4.

Indeed, Wikipedia is not authoritative, but it does provide a good
starting point to finding authoritative information if you want.

I suppose the authoritative doc is at
http://www.iana.org/assignments/multicast-addresses/multicast-addresses.xml
or somesuch.  It does not explicitly say 224.0.0.0/4 but it certainly
implies it in discussing the entire space from 224.0.0.0 through
239.255.255.255.

 diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
 index d5886fe..2b3b250 100644
 --- a/src/network/bridge_driver.c
 +++ b/src/network/bridge_driver.c
 @@ -1542,6 +1542,9 @@ networkRefreshDaemons(struct network_driver *driver)
  }
  }
  
 +static const char networkLocalMulticast[] = 224.0.0.0/24;

This should be /8, not /24.  There are many multicast protocols that
operate outside of that 224.0.0.0/24 network.  In fact they all must.
224.0.0.0/24 is reserved and assignment must be made from IANA.

Let me give you a scenario where your 224.0.0.0/24 fails to correct the
problem:

Imagine you have a cluster of VMs all with a network address of
192.168.122.[2-10]/24 and 192.168.122.1 is your VM server.  Now
imagine those VMs are using a multicast address of 226.1.2.3 (just for
argument's sake).  Each node will be listening on 226.1.2.3 for other
members of the group to build a list of members.

But once the packet from (say) 192.168.122.2-226.1.2.3 goes through the
VM host's NAT it will look to the other group members to be from host
192.168.122.1, so they all add 192.168.122.1 to their member list.  Then
192.168.122.3 sends it's packet to 226.1.2.3, it goes through NAT also
and all of the group members notice they already have 192.168.122.1 in
their group and just ignore it.  That goes on for all of the [2-10]
nodes and none of them get added to the group list.

That's because NAT broke the protocol.

Further, to your arguments in comment #13 in BZ #709418 about needing to
map ports  1023 for security:

The source port mapping goal has some specific parameters to it.  It's
trying to avoid a VM from masquerading as it's host and leveraging it's
host's permission in the network.  But that's only a problem because the
VM is masquerading as the host.  When you exempt multicast from
masquerading you no longer have the problem of the VM being mistaken for
the host and therefore you don't need the quasi-security of not
allowing the VM to use ports  1024.

Put another way, mapping the source port numbers is only a requirement
because the VM is borrowing the identity of the host.  Once you
eliminate the identity borrowing you can eliminate the need for the
safety guards that that lending is putting in place.

Cheers,
b.




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

Re: [libvirt] [PATCH 1/3] virsh-domain: Add --live, --config, --current logic to cmdAttachDevice

2013-05-28 Thread Guannan Ren

On 05/28/2013 06:52 PM, Peter Krempa wrote:

Use the approach established in commit
69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too.
---
  tools/virsh-domain.c | 41 -
  tools/virsh.pod  | 16 +---
  2 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0402aef..b965c11 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -173,13 +173,21 @@ static const vshCmdOptDef opts_attach_device[] = {
   .help = N_(XML file)
  },
  {.name = persistent,
- .type = VSH_OT_ALIAS,
- .help = config
+ .type = VSH_OT_BOOL,
+ .help = N_(make live change persistent)
  },
  {.name = config,
   .type = VSH_OT_BOOL,
   .help = N_(affect next boot)
  },
+{.name = live,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect running domain)
+},
+{.name = current,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect current domain)
+},
  {.name = NULL}
  };

@@ -191,7 +199,21 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
  char *buffer;
  int rv;
  bool ret = false;
-unsigned int flags;
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+bool current = vshCommandOptBool(cmd, current);
+bool config = vshCommandOptBool(cmd, config);
+bool live = vshCommandOptBool(cmd, live);
+bool persistent = vshCommandOptBool(cmd, persistent);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);



   VSH_EXCLUSIVE_OPTIONS_VAR(current, persistent)
   could be a little more consistent with the following two lines.




+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);


ACK.

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


Re: [libvirt] [PATCH 2/3] virsh-domain: Add --live, --config, --current logic to cmdAttachDisk

2013-05-28 Thread Guannan Ren

On 05/28/2013 06:52 PM, Peter Krempa wrote:

Use the approach established in commit
69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too.
---
  tools/virsh-domain.c | 53 
  tools/virsh.pod  | 13 +++--
  2 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b965c11..4de5dd5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -299,14 +299,6 @@ static const vshCmdOptDef opts_attach_disk[] = {
   .type = VSH_OT_STRING,
   .help = N_(mode of device reading and writing)
  },
-{.name = persistent,
- .type = VSH_OT_ALIAS,
- .help = config
-},
-{.name = config,
- .type = VSH_OT_BOOL,
- .help = N_(affect next boot)
-},
  {.name = sourcetype,
   .type = VSH_OT_STRING,
   .help = N_(type of source (block|file))
@@ -335,7 +327,22 @@ static const vshCmdOptDef opts_attach_disk[] = {
   .type = VSH_OT_BOOL,
   .help = N_(print XML document rather than attach the disk)
  },
-
+{.name = persistent,
+ .type = VSH_OT_BOOL,
+ .help = N_(make live change persistent)
+},
+{.name = config,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect next boot)
+},
+{.name = live,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect running domain)
+},
+{.name = current,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect current domain)
+},
  {.name = NULL}
  };

@@ -496,15 +503,33 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
  struct DiskAddress diskAddr;
  bool isFile = false, functionReturn = false;
  int ret;
-unsigned int flags;
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
  const char *stype = NULL;
  virBuffer buf = VIR_BUFFER_INITIALIZER;
  char *xml = NULL;
  struct stat st;
+bool current = vshCommandOptBool(cmd, current);
+bool config = vshCommandOptBool(cmd, config);
+bool live = vshCommandOptBool(cmd, live);
+bool persistent = vshCommandOptBool(cmd, persistent);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+


ACK either change or not^.

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


Re: [libvirt] [PATCH] util/vshstring: fix the vir_strdup when src is NULL.

2013-05-28 Thread Eric Blake
On 05/28/2013 01:13 AM, yangdongsheng wrote:
 When src is NULL, vir_strdup will return 0 directly.
 This patch will set dest to NULL before vir_strdup return.
 
 Signed-off-by: yangdongsheng yangds.f...@cn.fujitsu.com
 ---
  src/util/virstring.c |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

ACK, but VIR_STRNDUP has the same problem.  I'm pushing this with an
updated commit message (your followup mail that shows where it goes
wrong if not set to NULL) and with both problems fixed.

 
 diff --git a/src/util/virstring.c b/src/util/virstring.c
 index b244e6c..25b5d81 100644
 --- a/src/util/virstring.c
 +++ b/src/util/virstring.c
 @@ -540,8 +540,10 @@ virStrdup(char **dest,
const char *funcname,
size_t linenr)
  {
 -if (!src)
 +if (!src) {
 +*dest = NULL;
  return 0;
 +}
  if (!(*dest = strdup(src))) {
  if (report)
  virReportOOMErrorFull(domcode, filename, funcname, linenr);
 

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



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

Re: [libvirt] [PATCH 3/3] virsh-domain: Add --live, --config, --current logic to cmdAttachInterface

2013-05-28 Thread Guannan Ren

On 05/28/2013 06:52 PM, Peter Krempa wrote:

Use the approach established in commit
69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too.
---
  tools/virsh-domain.c | 59 +++-
  tools/virsh.pod  | 34 +-
  2 files changed, 60 insertions(+), 33 deletions(-)



-Attach a new network interface to the domain.
-Itype can be either Inetwork to indicate a physical network device or
-Ibridge to indicate a bridge to a device.
-Isource indicates the source device.
-Itarget allows to indicate the target device in the guest. Names starting
-with 'vnet' are considered as auto-generated an hence blanked out.
-Imac allows to specify the MAC address of the network interface.
-Iscript allows to specify a path to a script handling a bridge instead of
-the default one.
-Imodel allows to specify the model type.
-I--config indicates the changes will affect the next boot of the domain,
-for compatibility purposes, I--persistent is alias of I--config.
-Iinbound and Ioutbound control the bandwidth of the interface. Ipeak
-and Iburst are optional, so average,peak, average,,burst and
+Attach a new network interface to the domain.  Itype can be either Inetwork
+to indicate a physical network device or Ibridge to indicate a bridge to a
+device.  Isource indicates the source device.  Itarget allows to indicate
+the target device in the guest. Names starting with 'vnet' are considered as
+auto-generated an hence blanked out.


   Sorry the last sentence I can't quite follow it.

   ACK with the rest.

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


[libvirt] [PATCH] RPC: Support up to 4096 cpus on the host and 256 in the guest

2013-05-28 Thread Peter Krempa
The RPC limits for cpu maps didn't allow to use libvirt on ultra big
boxes. This patch increases size of the limits to support a maximum of
4096 cpus on the host with the built-in maximum of 256 cpus per guest.
The full cpu map of such a system takes 128 kilobytes and the map for
vcpu pinning is 512 bytes long.
---
 src/remote/remote_protocol.x | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 1ebbce7..a94e0db 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -82,13 +82,13 @@ const REMOTE_DOMAIN_ID_LIST_MAX = 16384;
 const REMOTE_DOMAIN_NAME_LIST_MAX = 16384;

 /* Upper limit on cpumap (bytes) passed to virDomainPinVcpu. */
-const REMOTE_CPUMAP_MAX = 256;
+const REMOTE_CPUMAP_MAX = 512;

 /* Upper limit on number of info fields returned by virDomainGetVcpus. */
-const REMOTE_VCPUINFO_MAX = 2048;
+const REMOTE_VCPUINFO_MAX = 4096;

 /* Upper limit on cpumaps (bytes) passed to virDomainGetVcpus. */
-const REMOTE_CPUMAPS_MAX = 16384;
+const REMOTE_CPUMAPS_MAX = 131072;

 /* Upper limit on migrate cookie. */
 const REMOTE_MIGRATE_COOKIE_MAX = 16384;
-- 
1.8.2.1

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


Re: [libvirt] [PATCH] RPC: Support up to 4096 cpus on the host and 256 in the guest

2013-05-28 Thread Eric Blake
On 05/28/2013 06:35 AM, Peter Krempa wrote:
 The RPC limits for cpu maps didn't allow to use libvirt on ultra big
 boxes. This patch increases size of the limits to support a maximum of
 4096 cpus on the host with the built-in maximum of 256 cpus per guest.
 The full cpu map of such a system takes 128 kilobytes and the map for
 vcpu pinning is 512 bytes long.
 ---
  src/remote/remote_protocol.x | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

We missed rc1 - is this still worth getting into 1.0.6?

I'm not opposed to the change, just questioning the timing of it.

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



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

Re: [libvirt] [libvirt-tck PATCH] 121-block-info.t: allow for greater capacity/allocation

2013-05-28 Thread Eric Blake
On 05/27/2013 01:35 PM, Guido Günther wrote:
 Don't be too picky about the actual size and allocation. The size needs
 to be correct but it's o.k. for allocation and size to be bigger. Debian
 Wheezy's qemu adds 4096 bytes.

Yeah, different versions of qemu round to different granularity; rather
annoying but that's life.

 ---
  scripts/domain/121-block-info.t |8 
  1 file changed, 4 insertions(+), 4 deletions(-)

ACK.

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



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

Re: [libvirt] [libvirt-tck PATCH] 121-block-info.t: omit network

2013-05-28 Thread Eric Blake
On 05/27/2013 01:17 PM, Guido Günther wrote:
 qemu:///session doesn't have a default network so we fail with:
 
  ./scripts/domain/121-block-info.t .. 1/29 # Defining transient storage pool
  # Generic guest with pervious created vol

Is this typo (s/pervious/previous/) something that also needs fixing in
source?

  ./scripts/domain/121-block-info.t .. 12/29
  #   Failed test 'Create domain'
  #   at 
 /var/lib/jenkins/jobs/libvirt-tck-build/workspace/lib//Sys/Virt/TCK.pm line 
 803.
  # expected Sys::Virt::Domain object
  # found 'libvirt error code: 43, message: Network not found: no network with 
 matching name 'default'
  # '
 
 The network isn't needed so simply leave it out.
 ---
  scripts/domain/121-block-info.t |1 -
  1 file changed, 1 deletion(-)

ACK.

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



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

Re: [libvirt] snapshot-create-as Permission denied

2013-05-28 Thread Eric Blake
On 05/26/2013 08:56 PM, yue wrote:

[please don't top-post on technical lists]

 hi.
 my environment: centos 6.3, qemu 1.5(source code build), libvirt 
 libvirt-0.10.2-18.el6_4.2.x86_64.selinux enforce .

It's best to use the entire stack from your distro, or to self-build the
entire stack.  Mixing newer qemu with older libvirt might have
unexpected consequences, and since you are using CentOS, you have no one
to blame but yourself.  We are unable to help you here unless you can
reproduce the problem with the latest libvirt.

  i have 2 questions
 1.snapshot. permisson deny.
 dumpxml:
  seclabel type='dynamic' model='selinux' relabel='yes'
 labelsystem_u:system_r:svirt_t:s0:c33,c172/label
 imagelabelsystem_u:object_r:svirt_image_t:s0:c33,c172/imagelabel
   /seclabel
 command line:
 [root@ovirtdev images]# ls -lZ
 -rw-r--r--. qemu qemu system_u:object_r:virt_image_t:s0 test.qcow2
 image does not have the same MLS?
 it does not seem like a selinux problem, because selinix does not record this 
 deny.

There have been bug fixes in upstream libvirt related to permissions on
snapshot creation, although it's hard to say whether all of those have
been backported into the downstream version of libvirt that you are using.

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



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

Re: [libvirt] [PATCH] RPC: Support up to 4096 cpus on the host and 256 in the guest

2013-05-28 Thread Peter Krempa

On 05/28/13 14:50, Eric Blake wrote:

On 05/28/2013 06:35 AM, Peter Krempa wrote:

The RPC limits for cpu maps didn't allow to use libvirt on ultra big
boxes. This patch increases size of the limits to support a maximum of
4096 cpus on the host with the built-in maximum of 256 cpus per guest.
The full cpu map of such a system takes 128 kilobytes and the map for
vcpu pinning is 512 bytes long.
---
  src/remote/remote_protocol.x | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)


We missed rc1 - is this still worth getting into 1.0.6?

I'm not opposed to the change, just questioning the timing of it.



I don't mind waiting until after the release to push this. Anyways this 
should be safe as we are just raising the limits and with some trickery 
I was able to verify that RPC is okay with transporting this amount of 
information this by reporting fake counts of cpus.


Peter

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


[libvirt] [PATCH] qemu: Don't report error on successful media eject

2013-05-28 Thread Cole Robinson
If we are just ejecting media, ret == -1 even after the retry loop
determines that the tray is open, as requested. This means media
disconnect always report's error.

Fix it, and fix some other mini issues:

- Don't overwrite the 'eject' error message if the retry loop fails
- Move the retries decrement inside the loop, otherwise the final loop
  might succeed, yet retries == 0 and we will raise error
- Setting ret = -1 in the disk-src check is unneeded
- Fix comment typos

cc: mpriv...@redhat.com
---
 src/qemu/qemu_hotplug.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1ed61db..3b277ef 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -98,10 +98,11 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
 
 virObjectRef(vm);
 /* we don't want to report errors from media tray_open polling */
-while (retries--) {
+while (retries) {
 if (origdisk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)
 break;
 
+retries--;
 virObjectUnlock(vm);
 VIR_DEBUG(Waiting 500ms for tray to open. Retries left %d, retries);
 usleep(500 * 1000); /* sleep 500ms */
@@ -109,19 +110,20 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr 
driver,
 }
 virObjectUnref(vm);
 
-if (disk-src) {
-/* deliberately don't depend on 'ret' as 'eject' may have failed for 
the
- * fist time and we are gonna check the drive state anyway */
-const char *format = NULL;
-
-/* We haven't succeeded yet */
-ret = -1;
-
-if (retries = 0) {
+if (retries = 0) {
+if (ret == 0) {
+/* If ret == -1, EjectMedia already set an error message */
 virReportError(VIR_ERR_OPERATION_FAILED, %s,
-   _(Unable to eject media before changing it));
-goto audit;
+   _(Unable to eject media));
 }
+goto audit;
+}
+ret = 0;
+
+if (disk-src) {
+/* deliberately don't depend on 'ret' as 'eject' may have failed the
+ * first time and we are gonna check the drive state anyway */
+const char *format = NULL;
 
 if (disk-type != VIR_DOMAIN_DISK_TYPE_DIR) {
 if (disk-format  0)
-- 
1.8.2.1

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


Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon

2013-05-28 Thread Eric Blake
On 05/25/2013 03:13 PM, Martin Kletzander wrote:
 @@ -31,6 +32,9 @@ foreach my $file (@ARGV) {
  while (defined (my $line = FILE)) {
  my $data = $line;

 +# Kill any quoted ; or 
 +$data =~ s,'[;]','X',g;
 +
 
 Good catch, I didn't know this was missing.

Yep, I hit some false positives where we parse for ';' if I didn't add this.

 
 [...]
 
 ACK,

Thanks; pushed.

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



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

Re: [libvirt] [PATCH v2] conf: Generate address for scsi host device automatically

2013-05-28 Thread John Ferlan
On 05/23/2013 12:48 PM, Osier Yang wrote:
 With unknown good reasons, the attribute bus of scsi device
 address is always set to 0, same for attribute target. (See
 virDomainDiskDefAssignAddress).
 
 Though we might need to change the algorithm to honor bus
 and target too, that's a different issue. The address generator
 
 for scsi host device in this patch just follows the unknown
 good reasons, only considering the controller and unit.
 It walks through all scsi controllers and their units, to see
 if the address $controller:0:0:$unit can be used, if found
 one, it sits on it, otherwise, it creates a new controller
 (actually the controller is implicitly created by someone
 else), and sits on $new_controller:0:0:0 instead.
 
 ---
 v1 - v2:
   * A new helper virDomainSCSIDriveAddressIsUsed
   * Move virDomainDefMaybeAddHostdevSCSIcontroller
   * More comments
   * Add testing.
   * problems fixed with the testing:
 1) s/nscsi_controllers + 1/nscsi_controllers/,
 2) Add the controller implicitly after a scsi hostdev def
parsing, as it can use a new scsi controller index
(nscsi_controllers), which should be added implicitly.

First pass of code is found at:

https://www.redhat.com/archives/libvir-list/2013-May/msg00340.html

The previous/initial review and reply is found at:

https://www.redhat.com/archives/libvir-list/2013-May/msg00506.html

https://www.redhat.com/archives/libvir-list/2013-May/msg00526.html

 ---
  src/conf/domain_conf.c | 209 
 ++---
  .../qemuxml2argv-hostdev-scsi-autogen-address.xml  |  95 ++
  ...qemuxml2xmlout-hostdev-scsi-autogen-address.xml | 106 +++
  tests/qemuxml2xmltest.c|   2 +
  4 files changed, 382 insertions(+), 30 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-autogen-address.xml
  create mode 100644 
 tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-autogen-address.xml
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index ad5550c..86d743b 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -3782,6 +3782,148 @@ cleanup:
  return ret;
  }
  
 +/* Check if a drive type address $controller:0:0:$unit is already
 + * taken by a disk or not.
 + */
 +static bool
 +virDomainDriveAddressIsUsedByDisk(virDomainDefPtr def,
 +  enum virDomainDiskBus type,
 +  unsigned int controller,
 +  unsigned int unit)
 +{
 +virDomainDiskDefPtr disk;
 +int i;
 +
 +for (i = 0; i  def-ndisks; i++) {
 +disk = def-disks[i];
 +
 +if (disk-bus != type ||
 +disk-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
 +continue;
 +
 +if (disk-info.addr.drive.controller == controller 
 +disk-info.addr.drive.unit == unit 
 +disk-info.addr.drive.bus == 0 
 +disk-info.addr.drive.target == 0)
 +return true;
 +}
 +
 +return false;
 +}
 +
 +/* Check if a drive type address $controller:0:0:$unit is already
 + * taken by a host device or not.
 + */
 +static bool
 +virDomainDriveAddressIsUsedByHostdev(virDomainDefPtr def,
 + enum virDomainHostdevSubsysType type,
 + unsigned int controller,
 + unsigned int unit)
 +{
 +virDomainHostdevDefPtr hostdev;
 +int i;
 +
 +for (i = 0; i  def-nhostdevs; i++) {
 +hostdev = def-hostdevs[i];
 +
 +if (hostdev-source.subsys.type != type)
 +continue;
 +
 +if (hostdev-info-addr.drive.controller == controller 
 +hostdev-info-addr.drive.unit == unit 
 +hostdev-info-addr.drive.bus == 0 
 +hostdev-info-addr.drive.target == 0)
 +return true;
 +}
 +
 +return false;
 +}
 +
 +static bool
 +virDomainSCSIDriveAddressIsUsed(virDomainDefPtr def,
 +unsigned int controller,
 +unsigned int unit)
 +{
 +/* In current implementation, the maximum unit number of a controller
 + * is either 16 or 7 (narrow SCSI bus), and if the maximum unit number
 + * is 16, the controller itself is on unit 7 */
 +if (unit == 7)


FYI: The previous code cared that a wide address was being used when
returning true - this code doesn't.  Just making sure that change is
intentional.  Essentially, 7 cannot be used.  The comments above the
code talk about wide/narrow which it seems becomes irrelevant. The
reality seems to be that you are avoiding using 7 altogether because
it's reserved for the controller itself, correct?  Regardless of whether
a wide or narrow.  The controller itself uses the unit number of the
maximum value of the narrow band.  At least that's how I'm reading it.

 +return true;
 +
 +if 

Re: [libvirt] [libvirt-tck PATCH] 121-block-info.t: omit network

2013-05-28 Thread Guido Günther
On Tue, May 28, 2013 at 06:56:21AM -0600, Eric Blake wrote:
 On 05/27/2013 01:17 PM, Guido Günther wrote:
  qemu:///session doesn't have a default network so we fail with:
  
   ./scripts/domain/121-block-info.t .. 1/29 # Defining transient storage pool
   # Generic guest with pervious created vol
 
 Is this typo (s/pervious/previous/) something that also needs fixing in
 source?
 
   ./scripts/domain/121-block-info.t .. 12/29
   #   Failed test 'Create domain'
   #   at 
  /var/lib/jenkins/jobs/libvirt-tck-build/workspace/lib//Sys/Virt/TCK.pm line 
  803.
   # expected Sys::Virt::Domain object
   # found 'libvirt error code: 43, message: Network not found: no network 
  with matching name 'default'
   # '
  
  The network isn't needed so simply leave it out.
  ---
   scripts/domain/121-block-info.t |1 -
   1 file changed, 1 deletion(-)
 
 ACK.

Both pushed including the typo fix. Thanks!
 -- Guido

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


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


Re: [libvirt] [PATCH 3/3] virsh-domain: Add --live, --config, --current logic to cmdAttachInterface

2013-05-28 Thread Peter Krempa

On 05/28/13 14:01, Guannan Ren wrote:

On 05/28/2013 06:52 PM, Peter Krempa wrote:

Use the approach established in commit
69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too.
---
  tools/virsh-domain.c | 59
+++-
  tools/virsh.pod  | 34 +-
  2 files changed, 60 insertions(+), 33 deletions(-)



-Attach a new network interface to the domain.
-Itype can be either Inetwork to indicate a physical network
device or
-Ibridge to indicate a bridge to a device.
-Isource indicates the source device.
-Itarget allows to indicate the target device in the guest. Names
starting
-with 'vnet' are considered as auto-generated an hence blanked out.
-Imac allows to specify the MAC address of the network interface.
-Iscript allows to specify a path to a script handling a bridge
instead of
-the default one.
-Imodel allows to specify the model type.
-I--config indicates the changes will affect the next boot of the
domain,
-for compatibility purposes, I--persistent is alias of I--config.
-Iinbound and Ioutbound control the bandwidth of the interface.
Ipeak
-and Iburst are optional, so average,peak, average,,burst and
+Attach a new network interface to the domain.  Itype can be either
Inetwork
+to indicate a physical network device or Ibridge to indicate a
bridge to a
+device.  Isource indicates the source device.  Itarget allows to
indicate
+the target device in the guest. Names starting with 'vnet' are
considered as
+auto-generated an hence blanked out.


Sorry the last sentence I can't quite follow it.


Hm. I actually didn't change the last sentence, I just reflowed the 
paragraph and deleted the sentence about --config and --persistent.


The last sentence means that if you use a interface name starting with 
'vnet' it will vanish as it's considered internal and autogenerated.




ACK with the rest.

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


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


Re: [libvirt] [PATCH] qemu: Don't report error on successful media eject

2013-05-28 Thread Eric Blake
On 05/28/2013 08:37 AM, Cole Robinson wrote:
 If we are just ejecting media, ret == -1 even after the retry loop
 determines that the tray is open, as requested. This means media
 disconnect always report's error.
 
 Fix it, and fix some other mini issues:
 
 - Don't overwrite the 'eject' error message if the retry loop fails
 - Move the retries decrement inside the loop, otherwise the final loop
   might succeed, yet retries == 0 and we will raise error
 - Setting ret = -1 in the disk-src check is unneeded
 - Fix comment typos
 
 cc: mpriv...@redhat.com
 ---
  src/qemu/qemu_hotplug.c | 26 ++
  1 file changed, 14 insertions(+), 12 deletions(-)

ACK.

 +
 +if (disk-src) {
 +/* deliberately don't depend on 'ret' as 'eject' may have failed the
 + * first time and we are gonna check the drive state anyway */

Comment motion, but might be worth s/gonna/going to/ while touching it
(I'm actually surprised that thunderbird didn't flag the slang 'gonna'
as a misspelled word).

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



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

Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon

2013-05-28 Thread Osier Yang

On 25/05/13 01:19, Eric Blake wrote:

Enforce the style cleanup in the previous patch.

* build-aux/bracket-spacing.pl: Enforce trailing spacing.
* cfg.mk (bracket-spacing-check): Tweak error wording.
* docs/hacking.html.in: Document the rule.
* HACKING: Regenerate.

Signed-off-by: Eric Blake ebl...@redhat.com
---
  HACKING  | 23 +++
  build-aux/bracket-spacing.pl | 12 
  cfg.mk   |  2 +-
  docs/hacking.html.in | 29 +
  4 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/HACKING b/HACKING
index 2bd6d69..42f76b6 100644
--- a/HACKING
+++ b/HACKING
@@ -318,6 +318,29 @@ immediately prior to any closing bracket. E.g.
int foo(int wizz);// Good


+Semicolons
+==
+Semicolons should never have a space beforehand. Inside the condition of a
+for loop, there should always be a space or line break after each semicolon,
+except for the special case of an infinite loop (although more infinite loops
+use while). While not enforced, loop counters generally use post-increment.
+
+  for (i = 0 ;i  limit ; ++i) { // Bad
+  for (i = 0; i  limit; i++) { // Good
+  for (;;) { // ok
+  while (1) { // Better
+
+Empty loop bodies are better represented with curly braces and a comment,
+although use of a semicolon is not currently rejected.
+
+  while ((rc = waitpid(pid, st, 0) == -1)  // ok
+ errno == EINTR);
+  while ((rc = waitpid(pid, st, 0) == -1)  // Better
+ errno == EINTR) {
+  /* nothing */
+  }
+
+
  Curly braces
  
  Omit the curly braces around an if, while, for etc. body only when that
diff --git a/build-aux/bracket-spacing.pl b/build-aux/bracket-spacing.pl
index 2b4..de40040 100755
--- a/build-aux/bracket-spacing.pl
+++ b/build-aux/bracket-spacing.pl
@@ -1,6 +1,7 @@
  #!/usr/bin/perl
  #
  # bracket-spacing.pl: Report any usage of 'function (..args..)'
+# Also check for other syntax issues, such as correct use of ';'
  #
  # This library is free software; you can redistribute it and/or
  # modify it under the terms of the GNU Lesser General Public
@@ -31,6 +32,9 @@ foreach my $file (@ARGV) {
  while (defined (my $line = FILE)) {
  my $data = $line;

+# Kill any quoted ; or 
+$data =~ s,'[;]','X',g;
+
  # Kill any quoted strings
  $data =~ s,([^\\\]|\\.)*,XXX,g;

@@ -125,6 +129,14 @@ foreach my $file (@ARGV) {
  $ret = 1;
  last;
  }
+
+# Require EOL, macro line continuation, or whitespace after :.
+# Allow for (;;) as an exception.
+while ($data =~ /;[^\\\n;)]/) {
+print $file:$.: $line;
+$ret = 1;
+last;
+}
  }
  close FILE;
  }
diff --git a/cfg.mk b/cfg.mk
index 55359e8..6e8b6d4 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -845,7 +845,7 @@ syntax-check: $(top_srcdir)/HACKING bracket-spacing-check
  bracket-spacing-check:
$(AM_V_GEN)files=`$(VC_LIST) | grep '\.c$$'`; \
$(PERL) $(top_srcdir)/build-aux/bracket-spacing.pl $$files || \
-  (echo $(ME): incorrect whitespace around brackets, see HACKING for rules 
 exit 1)
+  (echo $(ME): incorrect whitespace, see HACKING for rules  exit 1)


I see you changed this when pushing:

-  (echo $(ME): incorrect whitespace around brackets, see 
HACKING for rules  exit 1)

+ { echo $(ME): incorrect whitespace, see HACKING for rules 2; \
+   exit 1; }

But it breaks build. I guess you indented to write 21 there.

Osier

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


Re: [libvirt] [PATCH] qemu: Don't report error on successful media eject

2013-05-28 Thread Cole Robinson
On 05/28/2013 11:31 AM, Eric Blake wrote:
 On 05/28/2013 08:37 AM, Cole Robinson wrote:
 If we are just ejecting media, ret == -1 even after the retry loop
 determines that the tray is open, as requested. This means media
 disconnect always report's error.

 Fix it, and fix some other mini issues:

 - Don't overwrite the 'eject' error message if the retry loop fails
 - Move the retries decrement inside the loop, otherwise the final loop
   might succeed, yet retries == 0 and we will raise error
 - Setting ret = -1 in the disk-src check is unneeded
 - Fix comment typos

 cc: mpriv...@redhat.com
 ---
  src/qemu/qemu_hotplug.c | 26 ++
  1 file changed, 14 insertions(+), 12 deletions(-)
 
 ACK.
 
 +
 +if (disk-src) {
 +/* deliberately don't depend on 'ret' as 'eject' may have failed the
 + * first time and we are gonna check the drive state anyway */
 
 Comment motion, but might be worth s/gonna/going to/ while touching it
 (I'm actually surprised that thunderbird didn't flag the slang 'gonna'
 as a misspelled word).
 

Thanks, pushed with that comment fix.

- Cole

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


Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon

2013-05-28 Thread Eric Blake
On 05/28/2013 09:28 AM, Osier Yang wrote:
 On 25/05/13 01:19, Eric Blake wrote:
 Enforce the style cleanup in the previous patch.

 * build-aux/bracket-spacing.pl: Enforce trailing spacing.
 * cfg.mk (bracket-spacing-check): Tweak error wording.
 * docs/hacking.html.in: Document the rule.
 * HACKING: Regenerate.



 +  (echo $(ME): incorrect whitespace, see HACKING for rules
  exit 1)

 I see you changed this when pushing:
 
 -  (echo $(ME): incorrect whitespace around brackets, see
 HACKING for rules  exit 1)
 + { echo $(ME): incorrect whitespace, see HACKING for rules
 2; \
 +   exit 1; }
 
 But it breaks build. I guess you indented to write 21 there.

Phooey.  I was trying to copy how we did it elsewhere, and copied wrong
without testing my final edit.  I'll push the fix shortly.  :(

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



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

[libvirt] [PATCH] syntax: fix broken error message in previous patch

2013-05-28 Thread Eric Blake
Osier Yang pointed out that I introduced a syntax error in my
syntax check (I really shouldn't make last-minute changes without
testing them).

/bin/sh: -c: line 2: syntax error near unexpected token `;'
/bin/sh: -c: line 2: `  { echo 'maint.mk: incorrect whitespace, see HACKING for 
rules' 2; \'
make: *** [bracket-spacing-check] Error 1

* cfg.mk (bracket-spacing-check): Fix copy-and-paste error.
(sc_prohibit_duplicate_header): Use consistent spelling.

Signed-off-by: Eric Blake ebl...@redhat.com
---

Pushing under the build-breaker rule.

 cfg.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index a01df56..4ffa020 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -779,7 +779,7 @@ sc_prohibit_duplicate_header:
  }' $$i || fail=1; \
done;   \
if test $$fail -eq 1; then  \
- { echo $(ME): avoid duplicate headers 2; exit 1; }\
+ { echo '$(ME): avoid duplicate headers' 12; exit 1; }   \
fi;

 # Don't include libvirt/*.h in  form.
@@ -845,7 +845,7 @@ syntax-check: $(top_srcdir)/HACKING bracket-spacing-check
 bracket-spacing-check:
$(AM_V_GEN)files=`$(VC_LIST) | grep '\.c$$'`; \
$(PERL) $(top_srcdir)/build-aux/bracket-spacing.pl $$files || \
- { echo $(ME): incorrect whitespace, see HACKING for rules 2; \
+ { echo '$(ME): incorrect whitespace, see HACKING for rules' 12; \
exit 1; }

 # sc_po_check can fail if generated files are not built first
-- 
1.8.1.4

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


Re: [libvirt] [PATCH v2] conf: Generate address for scsi host device automatically

2013-05-28 Thread Osier Yang

On 28/05/13 23:09, John Ferlan wrote:

On 05/23/2013 12:48 PM, Osier Yang wrote:

With unknown good reasons, the attribute bus of scsi device
address is always set to 0, same for attribute target. (See
virDomainDiskDefAssignAddress).

Though we might need to change the algorithm to honor bus
and target too, that's a different issue. The address generator

for scsi host device in this patch just follows the unknown
good reasons, only considering the controller and unit.
It walks through all scsi controllers and their units, to see
if the address $controller:0:0:$unit can be used, if found
one, it sits on it, otherwise, it creates a new controller
(actually the controller is implicitly created by someone
else), and sits on $new_controller:0:0:0 instead.

---
v1 - v2:
   * A new helper virDomainSCSIDriveAddressIsUsed
   * Move virDomainDefMaybeAddHostdevSCSIcontroller
   * More comments
   * Add testing.
   * problems fixed with the testing:
 1) s/nscsi_controllers + 1/nscsi_controllers/,
 2) Add the controller implicitly after a scsi hostdev def
parsing, as it can use a new scsi controller index
(nscsi_controllers), which should be added implicitly.

First pass of code is found at:

https://www.redhat.com/archives/libvir-list/2013-May/msg00340.html

The previous/initial review and reply is found at:

https://www.redhat.com/archives/libvir-list/2013-May/msg00506.html

https://www.redhat.com/archives/libvir-list/2013-May/msg00526.html


---
  src/conf/domain_conf.c | 209 ++---
  .../qemuxml2argv-hostdev-scsi-autogen-address.xml  |  95 ++
  ...qemuxml2xmlout-hostdev-scsi-autogen-address.xml | 106 +++
  tests/qemuxml2xmltest.c|   2 +
  4 files changed, 382 insertions(+), 30 deletions(-)
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-autogen-address.xml
  create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-autogen-address.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ad5550c..86d743b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3782,6 +3782,148 @@ cleanup:
  return ret;
  }
  
+/* Check if a drive type address $controller:0:0:$unit is already

+ * taken by a disk or not.
+ */
+static bool
+virDomainDriveAddressIsUsedByDisk(virDomainDefPtr def,
+  enum virDomainDiskBus type,
+  unsigned int controller,
+  unsigned int unit)
+{
+virDomainDiskDefPtr disk;
+int i;
+
+for (i = 0; i  def-ndisks; i++) {
+disk = def-disks[i];
+
+if (disk-bus != type ||
+disk-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
+continue;
+
+if (disk-info.addr.drive.controller == controller 
+disk-info.addr.drive.unit == unit 
+disk-info.addr.drive.bus == 0 
+disk-info.addr.drive.target == 0)
+return true;
+}
+
+return false;
+}
+
+/* Check if a drive type address $controller:0:0:$unit is already
+ * taken by a host device or not.
+ */
+static bool
+virDomainDriveAddressIsUsedByHostdev(virDomainDefPtr def,
+ enum virDomainHostdevSubsysType type,
+ unsigned int controller,
+ unsigned int unit)
+{
+virDomainHostdevDefPtr hostdev;
+int i;
+
+for (i = 0; i  def-nhostdevs; i++) {
+hostdev = def-hostdevs[i];
+
+if (hostdev-source.subsys.type != type)
+continue;
+
+if (hostdev-info-addr.drive.controller == controller 
+hostdev-info-addr.drive.unit == unit 
+hostdev-info-addr.drive.bus == 0 
+hostdev-info-addr.drive.target == 0)
+return true;
+}
+
+return false;
+}
+
+static bool
+virDomainSCSIDriveAddressIsUsed(virDomainDefPtr def,
+unsigned int controller,
+unsigned int unit)
+{
+/* In current implementation, the maximum unit number of a controller
+ * is either 16 or 7 (narrow SCSI bus), and if the maximum unit number
+ * is 16, the controller itself is on unit 7 */
+if (unit == 7)


FYI: The previous code cared that a wide address was being used when
returning true - this code doesn't.  Just making sure that change is
intentional.  Essentially, 7 cannot be used.  The comments above the
code talk about wide/narrow which it seems becomes irrelevant. The
reality seems to be that you are avoiding using 7 altogether because
it's reserved for the controller itself, correct?  Regardless of whether
a wide or narrow.  The controller itself uses the unit number of the
maximum value of the narrow band.  At least that's how I'm reading it.


You are right, will update the comment.




+return true;
+
+if 

[libvirt] [PATCH] iscsi: support IPv6 targets

2013-05-28 Thread Ján Tomko
Instead of resolving the host as IPv4 and only using the first result,
resolve it as IPv6 as well and use the first address we've succesfully
connected to as the portal name.
---
 src/libvirt_private.syms|  1 +
 src/storage/storage_backend_iscsi.c | 64 +--
 src/util/virutil.c  | 87 +
 src/util/virutil.h  |  4 ++
 4 files changed, 102 insertions(+), 54 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9d5f74b..26c4553 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1931,6 +1931,7 @@ virEnumFromString;
 virEnumToString;
 virFindFCHostCapableVport;
 virFormatIntDecimal;
+virGetConnectableIPAsString;
 virGetDeviceID;
 virGetDeviceUnprivSGIO;
 virGetFCHostNameByWWN;
diff --git a/src/storage/storage_backend_iscsi.c 
b/src/storage/storage_backend_iscsi.c
index ad38ab2..f98fbce 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -1,7 +1,7 @@
 /*
  * storage_backend_iscsi.c: storage backend for iSCSI handling
  *
- * Copyright (C) 2007-2008, 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2007-2008, 2010-2013 Red Hat, Inc.
  * Copyright (C) 2007-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -44,58 +44,16 @@
 #include vircommand.h
 #include virrandom.h
 #include virstring.h
+#include virutil.h
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
-static int
-virStorageBackendISCSITargetIP(const char *hostname,
-   char *ipaddr,
-   size_t ipaddrlen)
-{
-struct addrinfo hints;
-struct addrinfo *result = NULL;
-int ret;
-
-memset(hints, 0, sizeof(hints));
-hints.ai_flags = AI_ADDRCONFIG;
-hints.ai_family = AF_INET;
-hints.ai_socktype = SOCK_STREAM;
-hints.ai_protocol = 0;
-
-ret = getaddrinfo(hostname, NULL, hints, result);
-if (ret != 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(host lookup failed %s),
-   gai_strerror(ret));
-return -1;
-}
-
-if (result == NULL) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(no IP address for target %s),
-   hostname);
-return -1;
-}
-
-if (getnameinfo(result-ai_addr, result-ai_addrlen,
-ipaddr, ipaddrlen, NULL, 0,
-NI_NUMERICHOST)  0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(cannot format ip addr for %s),
-   hostname);
-freeaddrinfo(result);
-return -1;
-}
-
-freeaddrinfo(result);
-return 0;
-}
-
 static char *
 virStorageBackendISCSIPortal(virStoragePoolSourcePtr source)
 {
-char ipaddr[NI_MAXHOST];
-char *portal;
+char *ip;
+char *portal = NULL;
+int port;
 
 if (source-nhost != 1) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
@@ -103,17 +61,15 @@ virStorageBackendISCSIPortal(virStoragePoolSourcePtr 
source)
 return NULL;
 }
 
-if (virStorageBackendISCSITargetIP(source-hosts[0].name,
-   ipaddr, sizeof(ipaddr))  0)
+port = source-hosts[0].port ? source-hosts[0].port : 3260;
+
+if (!(ip = virGetConnectableIPAsString(source-hosts[0].name, port)))
 return NULL;
 
-if (virAsprintf(portal, %s:%d,1, ipaddr,
-source-hosts[0].port ?
-source-hosts[0].port : 3260)  0) {
+if (virAsprintf(portal, %s,1, ip)  0)
 virReportOOMError();
-return NULL;
-}
 
+VIR_FREE(ip);
 return portal;
 }
 
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 028f1d1..d8400db 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -650,6 +650,93 @@ cleanup:
 return result;
 }
 
+/*
+ * Resolve 'host' and return the first address we've connected to
+ * as a string:
+ *   address:port
+ * for IPv4 addresses and
+ *   [address]:port
+ * for IPv6 addresses.
+ */
+char *
+virGetConnectableIPAsString(const char *host,
+int port)
+{
+struct addrinfo hints;
+struct addrinfo *res, *rp;
+int saved_errno = EINVAL;
+char *ret = NULL;
+char *buf = NULL;
+char *portstr = NULL;
+int rc;
+
+if (VIR_ALLOC_N(buf, NI_MAXHOST)  0)
+goto no_memory;
+
+if (virAsprintf(portstr, %d, port)  0)
+goto no_memory;
+
+memset(hints, 0, sizeof(hints));
+hints.ai_socktype = SOCK_STREAM;
+hints.ai_flags = AI_ADDRCONFIG;
+
+if ((rc = getaddrinfo(host, portstr, hints, res))) {
+virReportError(VIR_ERR_UNKNOWN_HOST,
+   _(unable to resolve '%s': %s),
+ host, gai_strerror(rc));
+goto cleanup;
+}
+
+for (rp = res; rp; rp = rp-ai_next) {
+int sockfd;
+
+sockfd = socket(rp-ai_family, rp-ai_socktype, 

Re: [libvirt] [PATCH v2] qemuOpenVhostNet: Decrease vhostfdSize on open failure

2013-05-28 Thread Laine Stump
On 05/27/2013 02:44 AM, Michal Privoznik wrote:
 Currently, if there's an error opening /dev/vhost-net (e.g. because
 it doesn't exist) but it's not required we proceed with vhostfd array
 filled with -1 and vhostfdSize unchanged. Later, when constructing
 the qemu command line only non-negative items within vhostfd array
 are taken into account. This means, vhostfdSize may be greater than
 the actual count of non-negative items in vhostfd array. This results
 in improper command line arguments being generated, e.g.:

 -netdev tap,fd=21,id=hostnet0,vhost=on,vhostfd=(null)
 ---
  src/qemu/qemu_command.c | 14 --
  1 file changed, 8 insertions(+), 6 deletions(-)

 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 0373626..c4a162a 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -486,6 +486,10 @@ qemuOpenVhostNet(virDomainDefPtr def,
 but is unavailable));
  goto error;
  }
 +VIR_WARN(Unable to open vhost-net. Opened so far %d, requested 
 %d,
 + i, *vhostfdSize);
 +*vhostfdSize = i;
 +break;
  }
  }
  virDomainAuditNetDevice(def, net, /dev/vhost-net, *vhostfdSize);
 @@ -6560,12 +6564,10 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
  }
  
  for (i = 0; i  vhostfdSize; i++) {
 -if (vhostfd[i] = 0) {
 -virCommandTransferFD(cmd, vhostfd[i]);
 -if (virAsprintf(vhostfdName[i], %d, vhostfd[i])  0) {
 -virReportOOMError();
 -goto cleanup;
 -}
 +virCommandTransferFD(cmd, vhostfd[i]);
 +if (virAsprintf(vhostfdName[i], %d, vhostfd[i])  0) {
 +virReportOOMError();
 +goto cleanup;
  }
  }
  

ACK.

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


[libvirt] Put the audio work on Windows 2008 Server 64bits

2013-05-28 Thread Carlos Rodrigues
Hello,

I am trying to put the audio audio work on Windows 2008 Server with
64bits but without success.

I have tested with sound model ac97 and hda, and i always get the same error: 
the devices appears on Windows Device Manager with following message - 
Windows cannot load the device driver for this hardware. 
The driver may be corrupted or missing. (Code 39).

I have qemu-kvm 1.2 and libvirt 0.10.2.

I also tried different drivers with different sound model combinations
but no success.

Any suggestion of drivers that can i use and how can i get audio device
working?

Best Regards,

-- 
Carlos Rodrigues c...@eurotux.com

Engenheiro de Software Sénior

Eurotux Informática, S.A. | www.eurotux.com

(t) +351 253 680 300





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 1/2] add hostdev passthrough common library

2013-05-28 Thread Laine Stump
On 05/27/2013 12:52 AM, Marek Marczykowski wrote:
 On 16.05.2013 08:07, Chunyan Liu wrote:
 Write separate module for hostdev passthrough so that it could be used by all
 hypervisor drivers and maintain a global hostdev state.

 Signed-off-by: Chunyan Liu cy...@suse.com
 Some comments inline. Besides that seems to be working (with your next patch
 for libxl) :)

 ---
  po/POTFILES.in   |1 +
  src/Makefile.am  |1 +
  src/libvirt.c|5 +
  src/libvirt_private.syms |   15 +
  src/util/virhostdevmanager.c | 1218 
 ++
  src/util/virhostdevmanager.h |   91 
  src/util/virpci.c|   17 +-
  src/util/virpci.h|7 +-
  src/util/virusb.c|   19 +-
  src/util/virusb.h|4 +-
  10 files changed, 1362 insertions(+), 16 deletions(-)
  create mode 100644 src/util/virhostdevmanager.c
  create mode 100644 src/util/virhostdevmanager.h

 diff --git a/po/POTFILES.in b/po/POTFILES.in
 index f3ea4da..a7c21bf 100644
 --- a/po/POTFILES.in
 +++ b/po/POTFILES.in
 @@ -188,6 +188,7 @@ src/util/viruri.c
  src/util/virusb.c
  src/util/virutil.c
  src/util/virxml.c
 +src/util/virhostdevmanager.c
  src/vbox/vbox_MSCOMGlue.c
  src/vbox/vbox_XPCOMCGlue.c
  src/vbox/vbox_driver.c
 diff --git a/src/Makefile.am b/src/Makefile.am
 index 4312c3c..a197c2b 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -130,6 +130,7 @@ UTIL_SOURCES =   
 \
  util/virutil.c util/virutil.h   \
  util/viruuid.c util/viruuid.h   \
  util/virxml.c util/virxml.h \
 +util/virhostdevmanager.c util/virhostdevmanager.h \
  $(NULL)
  
  
 diff --git a/src/libvirt.c b/src/libvirt.c
 index 2b3515e..d9af5a6 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -65,6 +65,7 @@
  #include virthread.h
  #include virstring.h
  #include virutil.h
 +#include virhostdevmanager.h
  
  #ifdef WITH_TEST
  # include test/test_driver.h
 @@ -827,6 +828,7 @@ int virStateInitialize(bool privileged,
  if (virInitialize()  0)
  return -1;
  
 +virHostdevManagerInit();
  for (i = 0 ; i  virStateDriverTabCount ; i++) {
  if (virStateDriverTab[i]-stateInitialize) {
  VIR_DEBUG(Running global init for %s state driver,
 @@ -858,6 +860,9 @@ int virStateCleanup(void) {
  virStateDriverTab[i]-stateCleanup()  0)
  ret = -1;
  }
 +
 +virHostdevManagerCleanup();
 + 
  return ret;
  }
  
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index cdd0b41..824de4e 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1991,6 +1991,21 @@ virXPathULong;
  virXPathULongHex;
  virXPathULongLong;
  
 +#util/virhostdevmanager.h
 +virHostdevManagerGetDefault;
 +virHostdevManagerInit;
 +virHostdevManagerCleanup;
 +virHostdevManagerPrepareHostdevs;
 +virHostdevManagerReAttachHostdevs;
 +virHostdevManagerPreparePciHostdevs;
 +virHostdevManagerPrepareUsbHostdevs;
 +virHostdevManagerReAttachPciHostdevs;
 +virHostdevManagerReAttachUsbHostdevs;
 +virGetActivePciHostdevs;
 +virGetActiveUsbHostdevs;
 +virGetDomainActivePciHostdevs;
 +virGetDomainActiveUsbHostdevs;
 +virFreeHostdevNameList;
 make check reports wrong symbol order here.

  
  # Let emacs know we want case-insensitive sorting
  # Local Variables:
 diff --git a/src/util/virhostdevmanager.c b/src/util/virhostdevmanager.c
 new file mode 100644
 index 000..9034212
 --- /dev/null
 +++ b/src/util/virhostdevmanager.c
 @@ -0,0 +1,1218 @@
 +/*
 + * Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
 + *
 + * This library is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU Lesser General Public
 + * License as published by the Free Software Foundation; either
 + * version 2.1 of the License, or (at your option) any later version.
 + *
 + * This library is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * Lesser General Public License for more details.
 + *
 + * You should have received a copy of the GNU Lesser General Public
 + * License along with this library.  If not, see
 + * http://www.gnu.org/licenses/.
 + *
 + * Author: Chunyan Liu cy...@suse.com
 + */
 +#include config.h
 +
 +#include virhostdevmanager.h
 +
 +#include sys/types.h
 +#include sys/stat.h
 +#include unistd.h
 +#include stdlib.h
 +#include stdio.h
 +
 +#include viralloc.h
 +#include virstring.h
 +#include virfile.h
 +#include virerror.h
 +#include virlog.h
 +#include virpci.h
 +#include virusb.h
 +#include virnetdev.h
 +
 +/* For virReportOOMError()  and virReportSystemError() */
 +#define VIR_FROM_THIS VIR_FROM_NONE
 +
 +struct _virHostdevManager{
 +char *stateDir;
 +
 +

Re: [libvirt] [PATCH] qemu: match vcpu topology to maximum vcpu numbers.

2013-05-28 Thread Eric Blake
On 05/28/2013 03:59 AM, Guannan Ren wrote:
 For qemu, if the -smp N or the value of maxcpus is given, it define
 the number of vcpu of guest whenever the vcpu topology is defined or
 not. But if the -smp N and maxcpus are missing, the topology can
 compute and define vcpus for guest automatically by math:
 
 vcpu number = sockets*cores*threads
 
 For libvirt, as vcpu is always mandatory, so we can ask topology
 to match maximum vcpu numbers.
 ---
  src/conf/domain_conf.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index a9656af..ffdc6da 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -11815,10 +11815,10 @@ virDomainDefParseXML(xmlDocPtr xml,
  goto error;
  
  if (def-cpu-sockets 
 -def-maxvcpus 
 +def-maxvcpus !=
  def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
  virReportError(VIR_ERR_XML_DETAIL, %s,
 -   _(Maximum CPUs greater than topology limit));
 +   _(Topology limit does not match maximum CPUs));

Is this going to reject XML that was previously accepted?  Is there a
bugzilla showing what happens if this patch is not incorporated?  I'm
worried about introducing an unintentional regression if we include this
in 1.0.6 without more justification.

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



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

Re: [libvirt] Put the audio work on Windows 2008 Server 64bits

2013-05-28 Thread Eric Blake
On 05/28/2013 11:06 AM, Carlos Rodrigues wrote:
 Hello,
 
 I am trying to put the audio audio work on Windows 2008 Server with
 64bits but without success.
 
 I have tested with sound model ac97 and hda, and i always get the same error: 
 the devices appears on Windows Device Manager with following message - 
 Windows cannot load the device driver for this hardware. 
 The driver may be corrupted or missing. (Code 39).
 
 I have qemu-kvm 1.2 and libvirt 0.10.2.
 
 I also tried different drivers with different sound model combinations
 but no success.
 
 Any suggestion of drivers that can i use and how can i get audio device
 working?

Have you considered using SPICE for your audio?  Personally, I haven't
tried audio passthrough to my one windows guest, so I don't have many
ideas; it may be better to ask on spice lists, precisely because that's
where you will find more developers that actually play with that sort of
thing.

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



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

[libvirt] [PATCH 3/3] virsh: migrate: Don't disallow --p2p and --migrateuri

2013-05-28 Thread Cole Robinson
Because it's a valid combination. p2p still uses a separate channel
for qemu migration, so there's value in letting the user specify a manual
migrate URI for overriding auto-port, or libvirt's FQDN lookup.

What _isn't_ allowed is --migrateuri and TUNNELLED, since there is
no separate migration channel. Disallow that instead
---
 tools/virsh-domain.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4fdf4ba..eb8688d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8306,15 +8306,15 @@ doMigrate(void *opaque)
 
 if ((flags  VIR_MIGRATE_PEER2PEER) ||
 vshCommandOptBool(cmd, direct)) {
-/* For peer2peer migration or direct migration we only expect one URI
- * a libvirt URI, or a hypervisor specific URI. */
 
-if (migrateuri != NULL) {
+/* migrateuri doesn't make sense for tunnelled migration */
+if (flags  VIR_MIGRATE_TUNNELLED  migrateuri != NULL) {
 vshError(ctl, %s, _(migrate: Unexpected migrateuri for 
peer2peer/direct migration));
 goto out;
 }
 
-if (virDomainMigrateToURI2(dom, desturi, NULL, xml, flags, dname, 0) 
== 0)
+if (virDomainMigrateToURI2(dom, desturi, migrateuri,
+   xml, flags, dname, 0) == 0)
 ret = '0';
 } else {
 /* For traditional live migration, connect to the destination host 
directly. */
-- 
1.8.2.1

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


[libvirt] [PATCH 1/3] qemu: migration: Improve p2p error if we can't open conn

2013-05-28 Thread Cole Robinson
By actually showing the Open() error to the user
---
 src/qemu/qemu_migration.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 19b1236..9ac9be4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3472,7 +3472,8 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver,
 qemuDomainObjExitRemote(vm);
 if (dconn == NULL) {
 virReportError(VIR_ERR_OPERATION_FAILED,
-   _(Failed to connect to remote libvirt URI %s), 
dconnuri);
+   _(Failed to connect to remote libvirt URI %s: %s),
+   dconnuri, virGetLastErrorMessage());
 virObjectUnref(cfg);
 return -1;
 }
-- 
1.8.2.1

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


[libvirt] [PATCH 2/3] qemu: migration: error if tunnelled + storage specified

2013-05-28 Thread Cole Robinson
Since as the code indicates it doesn't work yet, so let's be
explicit about it.
---
 src/qemu/qemu_migration.c | 24 ++--
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 9ac9be4..ffc86a4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1900,12 +1900,13 @@ char *qemuMigrationBegin(virQEMUDriverPtr driver,
 if (flags  (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) 
 virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NBD_SERVER)) {
 /* TODO support NBD for TUNNELLED migration */
-if (flags  VIR_MIGRATE_TUNNELLED)
-VIR_DEBUG(NBD in tunnelled migration is currently not supported);
-else {
-cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
-priv-nbdPort = 0;
+if (flags  VIR_MIGRATE_TUNNELLED) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+_(NBD in tunnelled migration is currently not supported));
+goto cleanup;
 }
+cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
+priv-nbdPort = 0;
 }
 
 if (!(mig = qemuMigrationEatCookie(driver, vm, NULL, 0, 0)))
@@ -2200,16 +2201,11 @@ done:
 if (mig-nbd 
 flags  (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) 
 virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NBD_SERVER)) {
-/* TODO support NBD for TUNNELLED migration */
-if (flags  VIR_MIGRATE_TUNNELLED)
-VIR_DEBUG(NBD in tunnelled migration is currently not supported);
-else {
-if (qemuMigrationStartNBDServer(driver, vm, listenAddr)  0) {
-/* error already reported */
-goto endjob;
-}
-cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
+if (qemuMigrationStartNBDServer(driver, vm, listenAddr)  0) {
+/* error already reported */
+goto endjob;
 }
+cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
 }
 
 if (qemuMigrationBakeCookie(mig, driver, vm, cookieout,
-- 
1.8.2.1

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


Re: [libvirt] [PATCH 1/3] qemu: migration: Improve p2p error if we can't open conn

2013-05-28 Thread Eric Blake
On 05/28/2013 02:44 PM, Cole Robinson wrote:
 By actually showing the Open() error to the user
 ---
  src/qemu/qemu_migration.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

Does qemuDomainObjExitRemote(vm) risk clobbering the last error message?
 Since it potentially unrefs vm on the last reference, I'm wondering if
we have to cache the last error at the right point in time instead of
relying on current behavior.  But obviously it worked in your testing as
written.

 
 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index 19b1236..9ac9be4 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -3472,7 +3472,8 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver,
  qemuDomainObjExitRemote(vm);
  if (dconn == NULL) {
  virReportError(VIR_ERR_OPERATION_FAILED,
 -   _(Failed to connect to remote libvirt URI %s), 
 dconnuri);
 +   _(Failed to connect to remote libvirt URI %s: %s),
 +   dconnuri, virGetLastErrorMessage());
  virObjectUnref(cfg);
  return -1;

Another alternative is to just return -1 directly instead of doing a
virReportError() that overwrites the original error, although I'm not
sure if that loses some context that would help the user.  Can you
include a sample error message in your commit log that shows the
improvement?  If so, I'm inclined to ACK this.

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



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

Re: [libvirt] [PATCH 2/3] qemu: migration: error if tunnelled + storage specified

2013-05-28 Thread Eric Blake
On 05/28/2013 02:44 PM, Cole Robinson wrote:
 Since as the code indicates it doesn't work yet, so let's be
 explicit about it.
 ---
  src/qemu/qemu_migration.c | 24 ++--
  1 file changed, 10 insertions(+), 14 deletions(-)

ACK.  An explicit error is always nicer than a nebulous todo that plows
on in unsupported state.

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



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

Re: [libvirt] [PATCH 3/3] virsh: migrate: Don't disallow --p2p and --migrateuri

2013-05-28 Thread Eric Blake
On 05/28/2013 02:44 PM, Cole Robinson wrote:
 Because it's a valid combination. p2p still uses a separate channel
 for qemu migration, so there's value in letting the user specify a manual
 migrate URI for overriding auto-port, or libvirt's FQDN lookup.
 
 What _isn't_ allowed is --migrateuri and TUNNELLED, since there is
 no separate migration channel. Disallow that instead
 ---
  tools/virsh-domain.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

ACK.  This exposes more power of the underlying API (the fact that it
only touches tools/virsh-domain.c is good).

 
 diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
 index 4fdf4ba..eb8688d 100644
 --- a/tools/virsh-domain.c
 +++ b/tools/virsh-domain.c
 @@ -8306,15 +8306,15 @@ doMigrate(void *opaque)
  
  if ((flags  VIR_MIGRATE_PEER2PEER) ||
  vshCommandOptBool(cmd, direct)) {
 -/* For peer2peer migration or direct migration we only expect one URI
 - * a libvirt URI, or a hypervisor specific URI. */
  
 -if (migrateuri != NULL) {
 +/* migrateuri doesn't make sense for tunnelled migration */
 +if (flags  VIR_MIGRATE_TUNNELLED  migrateuri != NULL) {
  vshError(ctl, %s, _(migrate: Unexpected migrateuri for 
 peer2peer/direct migration));
  goto out;
  }
  
 -if (virDomainMigrateToURI2(dom, desturi, NULL, xml, flags, dname, 0) 
 == 0)
 +if (virDomainMigrateToURI2(dom, desturi, migrateuri,
 +   xml, flags, dname, 0) == 0)
  ret = '0';
  } else {
  /* For traditional live migration, connect to the destination host 
 directly. */
 

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



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

[libvirt] [PATCH V2] Expose all CPU features in host definition

2013-05-28 Thread Don Dugger
[V2 - Improve the error handling]

I've opened BZ 697141 on this as I would consider it more
a bug than a feature request.  Anyway, to re-iterate my  
rationale from the BZ: 
  

The virConnectGetCapabilities API describes the host capabilities
by returning an XML description that includes the CPU model name 
and a set of CPU features.  The problem is that any features that
are part of the CPU model are not explicitly listed, they are
assumed to be part of the definition of that CPU model.  This
makes it extremely difficult for the caller of this API to check
for the presence of a specific CPU feature, the caller would have
to know what features are part of which CPU models, a very   
daunting task. 
  
This patch solves this problem by having the API return a model
name, as it currently does, but it will also explicitly list all
of the CPU features that are present.  This would make it much  
easier for a caller of this API to check for specific features.
   
Signed-off-by: Don Dugger donald.d.dug...@intel.com

-- 
 src/cpu/cpu_x86.c |   31 +++
 1 file changed, 31 insertions(+)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 5d479c2..098ab05 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1296,6 +1296,35 @@ x86GuestData(virCPUDefPtr host,
 return x86Compute(host, guest, data, message);
 }
 
+static int
+x86AddFeatures(virCPUDefPtr cpu,
+   struct x86_map *map)
+{
+int ret;
+const struct x86_model *candidate;
+const struct x86_feature *feature = map-features;
+
+candidate = map-models;
+while (candidate != NULL) {
+if (STREQ(cpu-model, candidate-name))
+break;
+candidate = candidate-next;
+}
+if (!candidate) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   Odd, %s not a known CPU model\n, cpu-model);
+return -1;
+}
+while (feature != NULL) {
+if (x86DataIsSubset(candidate-data, feature-data)) {
+if ((ret = virCPUDefAddFeature(cpu, feature-name, 
VIR_CPU_FEATURE_DISABLE))  0)
+return ret;
+}
+feature = feature-next;
+}
+return 0;
+}
+
 
 static int
 x86Decode(virCPUDefPtr cpu,
@@ -1383,6 +1412,8 @@ x86Decode(virCPUDefPtr cpu,
 goto out;
 }
 
+if (x86AddFeatures(cpuModel, map)  0)
+goto out;
 cpu-model = cpuModel-model;
 cpu-vendor = cpuModel-vendor;
 cpu-nfeatures = cpuModel-nfeatures;
-- 
1.7.10.4

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


Re: [libvirt] [PATCHv2] xml: introduce startupPolicy for chardev device

2013-05-28 Thread Eric Blake
On 05/24/2013 04:46 PM, Eric Blake wrote:
 From: Seiji Aguchi seiji.agu...@hds.com
 
 [Problem]
 Currently, guest OS's messages can be logged to a local disk of host OS
 by creating chadevs with options below.
   -chardev file,id=charserial0,path=log file's path -device 
 isa-serial,chardev=chardevserial0,id=serial0

 
 Seiji's patch plus my initial review comments; I've ack'd this much,
 but want to add a round-trip xml2xml test before pushing anything
 (Seiji, if you want to use this as a starting point, rather than
 waiting for my long weekend, feel free to post a v3).


Sorry, but this needs a v3, and I'm going to hand it back to you to fix.
 I tried adding a testsuite, but it proved that you weren't outputting
the startupPolicy during dumpxml formatting.  My initial attempt to add
it backfired (I'll post it below); I'm worried that we are playing too
fast and loose with a union type, since even with my patch in place,
'make check' fails with problems like:

84) QEMU XML-2-XML serial-dev ...
Offset 913
Expect [/
  target port='0'/
/serial
console type='dev'
  source path='/dev/ttyS2]
Actual [ startupPolicy='(null)'/
  target port='0'/
/serial
console type='dev'
  source path='/dev/ttyS2' startupPolicy='(null)]

which is a bug, since you said startupPolicy should only be tied to
files and not other serial types.  Do we need to repeat the
startupPolicy on the console mirror of the first serial device, or
is that unnecessary back-compat, and where we can safely declare that
startupPolicy is only emitted for the serial side of things?

All told, your patch is not complete until it passes 'make check' with a
new xml2xml test that proves we can round trip the new policy.

diff --git c/src/conf/domain_conf.c w/src/conf/domain_conf.c
index c24a9f0..5af5e40 100644
--- c/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -14547,8 +14547,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
 if (def-type != VIR_DOMAIN_CHR_TYPE_PTY ||
 (def-data.file.path 
  !(flags  VIR_DOMAIN_XML_INACTIVE))) {
-virBufferEscapeString(buf,   source path='%s'/\n,
+virBufferEscapeString(buf,   source path='%s',
   def-data.file.path);
+
+if (def-data.file.path  def-data.file.startupPolicy) {
+const char *policy =
virDomainStartupPolicyTypeToString(def-data.file.startupPolicy);
+virBufferAsprintf(buf,  startupPolicy='%s', policy);
+}
+virBufferAddLit(buf, /\n);
 }
 break;

diff --git
c/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args
w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args
new file mode 100644
index 000..24977f2
--- /dev/null
+++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args
@@ -0,0 +1,8 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu
-S -M \
+pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi -boot c -usb -hdc /tmp/idedisk.img \
+-chardev file,id=charserial0,path=/tmp/serial.log \
+-device isa-serial,chardev=charserial0,id=serial0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git
c/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml
w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml
new file mode 100644
index 000..34e0eb9
--- /dev/null
+++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml
@@ -0,0 +1,35 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219136/memory
+  currentMemory unit='KiB'219136/currentMemory
+  vcpu placement='static'1/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='file' device='disk'
+  source file='/tmp/idedisk.img'/
+  target dev='hdc' bus='ide'/
+  address type='drive' controller='0' bus='0' target='0' unit='2'/
+/disk
+controller type='usb' index='0'/
+controller type='ide' index='0'/
+controller type='pci' index='0' model='pci-root'/
+serial type='file'
+  source path='/tmp/serial.log' startupPolicy='optional'/
+  target port='0'/
+/serial
+console type='file'
+  source path='/tmp/serial.log'/
+  target type='serial' port='0'/
+/console
+memballoon model='virtio'/
+  /devices
+/domain
diff --git c/tests/qemuxml2argvtest.c w/tests/qemuxml2argvtest.c
index 2c7fd01..22f4782 100644
--- c/tests/qemuxml2argvtest.c
+++ w/tests/qemuxml2argvtest.c
@@ -727,6 +727,8 @@ mymain(void)
 QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, 

[libvirt] [PATCH] build: drop unused variable

2013-05-28 Thread Eric Blake
Compilation for mingw failed:

../../src/util/virutil.c: In function 'virGetWin32DirectoryRoot':
../../src/util/virutil.c:1094:9: error: unused variable 'ret' 
[-Werror=unused-variable]

* src/util/virutil.c (virGetWin32DirectoryRoot): Silence compiler
warning.

Signed-off-by: Eric Blake ebl...@redhat.com
---

Compilation still fails in the tests directory, due to the use of
fprintf(stderr, %zu, size_t); I'm still debating whether I can
get it fixed in gnulib and take the risk of a gnulib backport before
rc2, or whether we'll have to come up with some other less-invasive
workaround.

 src/util/virutil.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 028f1d1..a29da14 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1091,7 +1091,6 @@ static int
 virGetWin32DirectoryRoot(char **path)
 {
 char windowsdir[MAX_PATH];
-int ret = 0;

 *path = NULL;

-- 
1.8.1.4

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


Re: [libvirt] [PATCH] build: drop unused variable

2013-05-28 Thread Eric Blake
On 05/28/2013 05:20 PM, Eric Blake wrote:
 Compilation for mingw failed:
 
 ../../src/util/virutil.c: In function 'virGetWin32DirectoryRoot':
 ../../src/util/virutil.c:1094:9: error: unused variable 'ret' 
 [-Werror=unused-variable]
 
 * src/util/virutil.c (virGetWin32DirectoryRoot): Silence compiler
 warning.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---

I pushed this under the build-breaker rule, by the way.

 
 Compilation still fails in the tests directory, due to the use of
 fprintf(stderr, %zu, size_t); I'm still debating whether I can
 get it fixed in gnulib and take the risk of a gnulib backport before
 rc2, or whether we'll have to come up with some other less-invasive
 workaround.

Dan already posted a potential workaround, but he put it on hold pending
whatever I can determine on the gnulib side of things.

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



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

[libvirt] [PATCH] build: fix build with older gcc

2013-05-28 Thread Eric Blake
gcc 4.1.2 (hello, RHEL 5!) fails to build on 32-bit platforms with:

conf/domain_conf.c: In function 'virDomainDefParseXML':
conf/domain_conf.c:10581: warning: integer constant is too large for 'long' type

* src/conf/domain_conf.c (virDomainDefParseXML): Mark large constants.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/conf/domain_conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a9656af..2b4e160 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10578,7 +10578,7 @@ virDomainDefParseXML(xmlDocPtr xml,

 if (def-cputune.quota  0 
 (def-cputune.quota  1000 ||
- def-cputune.quota  18446744073709551)) {
+ def-cputune.quota  18446744073709551LL)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Value of cputune quota must be in range 
  [1000, 18446744073709551]));
@@ -10610,7 +10610,7 @@ virDomainDefParseXML(xmlDocPtr xml,

 if (def-cputune.emulator_quota  0 
 (def-cputune.emulator_quota  1000 ||
- def-cputune.emulator_quota  18446744073709551)) {
+ def-cputune.emulator_quota  18446744073709551LL)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Value of cputune emulator_quota must be in range 
  [1000, 18446744073709551]));
-- 
1.8.1.4

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


Re: [libvirt] [PATCH] build: fix build with older gcc

2013-05-28 Thread Eric Blake
On 05/28/2013 05:33 PM, Eric Blake wrote:
 gcc 4.1.2 (hello, RHEL 5!) fails to build on 32-bit platforms with:
 
 conf/domain_conf.c: In function 'virDomainDefParseXML':
 conf/domain_conf.c:10581: warning: integer constant is too large for 'long' 
 type
 
 * src/conf/domain_conf.c (virDomainDefParseXML): Mark large constants.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  src/conf/domain_conf.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Introduced in v1.0.4-6-gf8e3221; pushed under build-breaker rule.

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



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

Re: [libvirt] [PATCHv2] xml: introduce startupPolicy for chardev device

2013-05-28 Thread Seiji Aguchi
Eric,


 -Original Message-
 From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] 
 On Behalf Of Eric Blake
 Sent: Tuesday, May 28, 2013 7:08 PM
 Cc: libvir-list@redhat.com; dle-deve...@lists.sourceforge.net; Tomoki Sekiyama
 Subject: Re: [libvirt] [PATCHv2] xml: introduce startupPolicy for chardev 
 device
 
 On 05/24/2013 04:46 PM, Eric Blake wrote:
  From: Seiji Aguchi seiji.agu...@hds.com
 
  [Problem]
  Currently, guest OS's messages can be logged to a local disk of host OS
  by creating chadevs with options below.
-chardev file,id=charserial0,path=log file's path -device 
  isa-serial,chardev=chardevserial0,id=serial0
 
 
  Seiji's patch plus my initial review comments; I've ack'd this much,
  but want to add a round-trip xml2xml test before pushing anything
  (Seiji, if you want to use this as a starting point, rather than
  waiting for my long weekend, feel free to post a v3).
 
 
 Sorry, but this needs a v3, and I'm going to hand it back to you to fix.
  I tried adding a testsuite, but it proved that you weren't outputting
 the startupPolicy during dumpxml formatting.  My initial attempt to add
 it backfired (I'll post it below); 

Thank you for handling/testing my patch.

I'm worried that we are playing too
 fast and loose with a union type, since even with my patch in place,
 'make check' fails with problems like:
 
 84) QEMU XML-2-XML serial-dev ...
 Offset 913
 Expect [/
   target port='0'/
 /serial
 console type='dev'
   source path='/dev/ttyS2]
 Actual [ startupPolicy='(null)'/
   target port='0'/
 /serial
 console type='dev'
   source path='/dev/ttyS2' startupPolicy='(null)]
 
 which is a bug, since you said startupPolicy should only be tied to
 files and not other serial types.  Do we need to repeat the
 startupPolicy on the console mirror of the first serial device, or
 is that unnecessary back-compat, and where we can safely declare that
 startupPolicy is only emitted for the serial side of things?
 

I need to think carefully about it before deciding it in a hurry.

 All told, your patch is not complete until it passes 'make check' with a
 new xml2xml test that proves we can round trip the new policy.

Anyway, I will post the v3 patch by fixing the issue.

Seiji

 
 diff --git c/src/conf/domain_conf.c w/src/conf/domain_conf.c
 index c24a9f0..5af5e40 100644
 --- c/src/conf/domain_conf.c
 +++ w/src/conf/domain_conf.c
 @@ -14547,8 +14547,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
  if (def-type != VIR_DOMAIN_CHR_TYPE_PTY ||
  (def-data.file.path 
   !(flags  VIR_DOMAIN_XML_INACTIVE))) {
 -virBufferEscapeString(buf,   source path='%s'/\n,
 +virBufferEscapeString(buf,   source path='%s',
def-data.file.path);
 +
 +if (def-data.file.path  def-data.file.startupPolicy) {
 +const char *policy =
 virDomainStartupPolicyTypeToString(def-data.file.startupPolicy);
 +virBufferAsprintf(buf,  startupPolicy='%s', policy);
 +}
 +virBufferAddLit(buf, /\n);
  }
  break;
 
 diff --git
 c/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args
 w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args
 new file mode 100644
 index 000..24977f2
 --- /dev/null
 +++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args
 @@ -0,0 +1,8 @@
 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu
 -S -M \
 +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
 +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \
 +-mon chardev=charmonitor,id=monitor,mode=readline \
 +-no-acpi -boot c -usb -hdc /tmp/idedisk.img \
 +-chardev file,id=charserial0,path=/tmp/serial.log \
 +-device isa-serial,chardev=charserial0,id=serial0 \
 +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
 diff --git
 c/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml
 w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml
 new file mode 100644
 index 000..34e0eb9
 --- /dev/null
 +++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml
 @@ -0,0 +1,35 @@
 +domain type='qemu'
 +  nameQEMUGuest1/name
 +  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
 +  memory unit='KiB'219136/memory
 +  currentMemory unit='KiB'219136/currentMemory
 +  vcpu placement='static'1/vcpu
 +  os
 +type arch='i686' machine='pc'hvm/type
 +boot dev='hd'/
 +  /os
 +  clock offset='utc'/
 +  on_poweroffdestroy/on_poweroff
 +  on_rebootrestart/on_reboot
 +  on_crashdestroy/on_crash
 +  devices
 +emulator/usr/bin/qemu/emulator
 +disk type='file' device='disk'
 +  source file='/tmp/idedisk.img'/
 +  target dev='hdc' bus='ide'/
 +  address type='drive' controller='0' bus='0' target='0' unit='2'/
 +/disk
 +controller type='usb' index='0'/
 +

Re: [libvirt] [PATCH] qemu: match vcpu topology to maximum vcpu numbers.

2013-05-28 Thread Guannan Ren

On 05/29/2013 04:20 AM, Eric Blake wrote:

On 05/28/2013 03:59 AM, Guannan Ren wrote:

For qemu, if the -smp N or the value of maxcpus is given, it define
the number of vcpu of guest whenever the vcpu topology is defined or
not. But if the -smp N and maxcpus are missing, the topology can
compute and define vcpus for guest automatically by math:

vcpu number = sockets*cores*threads

For libvirt, as vcpu is always mandatory, so we can ask topology
to match maximum vcpu numbers.
---
  src/conf/domain_conf.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a9656af..ffdc6da 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11815,10 +11815,10 @@ virDomainDefParseXML(xmlDocPtr xml,
  goto error;
  
  if (def-cpu-sockets 

-def-maxvcpus 
+def-maxvcpus !=
  def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
  virReportError(VIR_ERR_XML_DETAIL, %s,
-   _(Maximum CPUs greater than topology limit));
+   _(Topology limit does not match maximum CPUs));

Is this going to reject XML that was previously accepted?  Is there a
bugzilla showing what happens if this patch is not incorporated?  I'm
worried about introducing an unintentional regression if we include this
in 1.0.6 without more justification.




Yes, it make the cpu topology limit setting more strict.
Alternatively, we can add more comments for topology to make it clear that
vcpu decide the number of vcpus,  the  sockets*cores*threads had better
be equal to vcpu numbers.


The bz:https://bugzilla.redhat.com/show_bug.cgi?id=880017

Description of problem:
libvirt should check if vcpu topology is right. If the wrong vcpu topology is 
given in xml , the wrong arguments also be passed to qemu-kvm.

vcpu number = sockets*cores*threads


Steps to Reproduce:
1.# virsh start vm
Domain vm started

2.# virsh dumpxml vm
domain type='kvm' id='104'
  ...
  vcpu placement='static'4/vcpu
   ..
  cpu
topology sockets='1' cores='4' threads='2'/
  /cpu


3.# ps -ef|grep qemu-kvm
qemu 21296 1 14 16:41 ?00:00:17 /usr/libexec/qemu-kvm -name vm 
-S -M rhel6.4.0 -enable-kvm -m 1024 -smp 4,sockets=1,cores=4,threads=2
..

  
Actual results:

Wrong vcpu topology can be given in xml and passed to qemu-kvm

Expected results:
libvirt should check if vcpu topology is right.






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


[libvirt] [PATCH] doc: cpu topology values should be equal to maxmium vcpus

2013-05-28 Thread Guannan Ren
---
 docs/formatdomain.html.in | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 755d084..27c1580 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -941,7 +941,9 @@
 virtual CPU provided to the guest. Three non-zero values have to be
 given for codesockets/code, codecores/code, and
 codethreads/code: total number of CPU sockets, number of cores per
-socket, and number of threads per core, respectively./dd
+socket, and number of threads per core, respectively. The maximum 
number
+of virtual CPUs is defined by codevcpu/code element, the product of
+the three valuses should be equal to the maximum vcpus./dd
 
   dtcodefeature/code/dt
   ddThe codecpu/code element can contain zero or more
-- 
1.8.1.4

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


Re: [libvirt] [PATCH] qemu: match vcpu topology to maximum vcpu numbers.

2013-05-28 Thread Eric Blake
On 05/28/2013 07:32 PM, Guannan Ren wrote:

 if (def-cpu-sockets 
 -def-maxvcpus 
 +def-maxvcpus !=
   def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
   virReportError(VIR_ERR_XML_DETAIL, %s,
 -   _(Maximum CPUs greater than topology
 limit));
 +   _(Topology limit does not match maximum
 CPUs));
 Is this going to reject XML that was previously accepted?  Is there a
 bugzilla showing what happens if this patch is not incorporated?  I'm
 worried about introducing an unintentional regression if we include this
 in 1.0.6 without more justification.

 
 
 Yes, it make the cpu topology limit setting more strict.

I'm okay with making things more strict, if it makes sense.  But isn't
it feasible to support a topology that has more slots than the maximum?
 That is, since a bare metal machine can have two sockets but only
populate one, why can't a virtual machine have 2 sockets but set a max
vcpu of 1 so that only one socket can be populated?  Does qemu itself
enforce that there be a match?

That's why I'm worried about enforcing that the user can't have a
mismatch between maxvcpus vs. topology, if they specify both.  I'm also
worried that this check might fire if the user specifies only topology
or only maxvcpus; I want to make sure that we only do the sanity
checking if both numbers are provided.

 Alternatively, we can add more comments for topology to make it clear
 that
 vcpu decide the number of vcpus,  the  sockets*cores*threads had better
 be equal to vcpu numbers.

If there is a problem with a mismatch, then we should document that a
match is required, and fail on mismatch.  Merely documenting that a
match is required without enforcing it is too weak; and enforcing it
without documentation is a disservice.  I haven't ruled out enforcing a
match, but want to make sure that we aren't artificially crippling
something that might prove useful in reality, when compared with what
bare-metal setups can provide.

 
 
 The bz:https://bugzilla.redhat.com/show_bug.cgi?id=880017
 
 Description of problem:
 libvirt should check if vcpu topology is right. If the wrong vcpu
 topology is given in xml , the wrong arguments also be passed to qemu-kvm.
 
 vcpu number = sockets*cores*threads
 
 
 Steps to Reproduce:
 1.# virsh start vm
 Domain vm started
 
 2.# virsh dumpxml vm
 domain type='kvm' id='104'
   ...
   vcpu placement='static'4/vcpu
..
   cpu
 topology sockets='1' cores='4' threads='2'/
   /cpu
 
 
 3.# ps -ef|grep qemu-kvm
 qemu 21296 1 14 16:41 ?00:00:17 /usr/libexec/qemu-kvm
 -name vm -S -M rhel6.4.0 -enable-kvm -m 1024 -smp
 4,sockets=1,cores=4,threads=2
 ..
 
   Actual results:
 Wrong vcpu topology can be given in xml and passed to qemu-kvm
 
 Expected results:
 libvirt should check if vcpu topology is right.
 
 
 
 
 
 
 
 

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



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

Re: [libvirt] [PATCH] doc: cpu topology values should be equal to maxmium vcpus

2013-05-28 Thread Eric Blake
On 05/28/2013 08:53 PM, Guannan Ren wrote:
 ---
  docs/formatdomain.html.in | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 755d084..27c1580 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -941,7 +941,9 @@
  virtual CPU provided to the guest. Three non-zero values have to be
  given for codesockets/code, codecores/code, and
  codethreads/code: total number of CPU sockets, number of cores 
 per
 -socket, and number of threads per core, respectively./dd
 +socket, and number of threads per core, respectively. The maximum 
 number
 +of virtual CPUs is defined by codevcpu/code element, the product 
 of
 +the three valuses should be equal to the maximum vcpus./dd

s/valuses/values/

Again, if we are going to document that they must be equal, then the
code should match; otherwise, if we are going to allow a difference
between the two, then the wording should be a bit nicer (It is
recommended that if both maxvcpu and topology are provided, then the two
numbers should match).  Or maybe a one-sided restriction makes sense,
where maxvcpus can be less than or equal, but not greater than, an
explicit topology.

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



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

[libvirt] [PATCH] build: fix build with newer gnutls

2013-05-28 Thread Eric Blake
Building with gnutls 3.2.0 (such as shipped with current cygwin) fails
with:

rpc/virnettlscontext.c: In function 'virNetTLSSessionGetKeySize':
rpc/virnettlscontext.c:1358:5: error: implicit declaration of function 
'gnutls_cipher_get_key_size' [-Wimplicit-function-declaration]

Yeah, it's stupid that gnutls broke API by moving their declaration
into a new header without including that header from the old one,
but it's easy enough to work around, all without breaking on gnutls
1.4.1 (hello RHEL 5) that lacked the new header.

* src/rpc/virnettlscontext.c (includes): Include additional header.
* configure.ac (gnutls): Check for gnutls/crypto.h.

Signed-off-by: Eric Blake ebl...@redhat.com
---

Pushing under the build-breaker rule, once my testing completes
across multiple systems with various gnutls versions (1.4.1 in
RHEL5, 2.12.23 in Fedora 18, 3.2.0 in Cygwin).

 configure.ac   | 1 +
 src/rpc/virnettlscontext.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/configure.ac b/configure.ac
index 7041710..0b3d028 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1087,6 +1087,7 @@ if test x$with_gnutls != xno; then
   if test $GNUTLS_FOUND = no; then
 fail=0
 AC_CHECK_HEADER([gnutls/gnutls.h], [], [fail=1])
+AC_CHECK_HEADERS([gnutls/crypto.h])
 AC_CHECK_LIB([gnutls], [gnutls_handshake],[], [fail=1], [-lgcrypt])

 test $fail = 0  GNUTLS_FOUND=yes
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index e29c439..f2ac551 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -25,6 +25,9 @@
 #include stdlib.h

 #include gnutls/gnutls.h
+#if HAVE_GNULTLS_CRYPTO_H
+# include gnutls/crypto.h
+#endif
 #include gnutls/x509.h
 #include gnutls_1_0_compat.h

-- 
1.8.1.4

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


Re: [libvirt] [PATCH] build: fix build with newer gnutls

2013-05-28 Thread Eric Blake
On 05/28/2013 09:23 PM, Eric Blake wrote:
 Building with gnutls 3.2.0 (such as shipped with current cygwin) fails
 with:
 
 rpc/virnettlscontext.c: In function 'virNetTLSSessionGetKeySize':
 rpc/virnettlscontext.c:1358:5: error: implicit declaration of function 
 'gnutls_cipher_get_key_size' [-Wimplicit-function-declaration]
 
 Yeah, it's stupid that gnutls broke API by moving their declaration
 into a new header without including that header from the old one,
 but it's easy enough to work around, all without breaking on gnutls
 1.4.1 (hello RHEL 5) that lacked the new header.
 
 * src/rpc/virnettlscontext.c (includes): Include additional header.
 * configure.ac (gnutls): Check for gnutls/crypto.h.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
 
 Pushing under the build-breaker rule, once my testing completes
 across multiple systems with various gnutls versions (1.4.1 in
 RHEL5, 2.12.23 in Fedora 18, 3.2.0 in Cygwin).

Or not - it failed my builds.

 +++ b/configure.ac
 @@ -1087,6 +1087,7 @@ if test x$with_gnutls != xno; then
if test $GNUTLS_FOUND = no; then
  fail=0
  AC_CHECK_HEADER([gnutls/gnutls.h], [], [fail=1])
 +AC_CHECK_HEADERS([gnutls/crypto.h])

This line needs tweaking to pre-include gnutls/gnutls.h before probing
for crypto.h.  I'll post a v2 once I complete more testing.

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



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