Re: [libvirt] virsh bash completion file

2011-10-06 Thread Daniel P. Berrange 
On Wed, Oct 05, 2011 at 03:17:47PM -0500, Serge E. Hallyn wrote:
 Hi,
 
 I've been trying out a bash autocompletion file by Geoff Low (slight hack
 by me, don't blame him for my hack), and it's working pretty nicely.
 I'm not sure where to put it in the git tree, but it seems like it'd be
 nice to have upstream?

David Lutterkort previously suggested this

   https://www.redhat.com/archives/libvir-list/2007-October/msg00231.html

And I didn't ever notice that and so wrote one myself

   https://www.redhat.com/archives/libvir-list/2008-July/msg00175.html
   https://www.redhat.com/archives/libvir-list/2008-July/msg00177.html

Neither ever got committed. There were some things I didn't much like
about mine, but I can't really remember now.

Third time lucky perhaps :-)


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] virsh bash completion file

2011-10-06 Thread Daniel P. Berrange 
On Wed, Oct 05, 2011 at 02:33:44PM -0600, Eric Blake wrote:
 On 10/05/2011 02:17 PM, Serge E. Hallyn wrote:
 Hi,
 
 I've been trying out a bash autocompletion file by Geoff Low (slight hack
 by me, don't blame him for my hack), and it's working pretty nicely.
 I'm not sure where to put it in the git tree, but it seems like it'd be
 nice to have upstream?
 
 What I'd rather have upstream is:
 
 virsh completion args...
 
 which outputs one string per line of valid completions given the
 context of args.  Then we could leverage a single completion code
 both from bash and from the virsh interactive shell; also, you'd
 only have to write the bash completion routines once (figure out how
 to make it call into 'virsh completion'), rather than chasing a
 moving target (update the bash completion every time new virsh
 commands and flags are added).

The patch I wrote tried todo something similar to that,
adding a bunch of hidden '_complete-XXX'  commands.

https://www.redhat.com/archives/libvir-list/2008-July/msg00175.html

it was still a little bit too manual for my liking though.
It'd be nice if we could do a completion command that is
properly metadata driven from the command arg type data

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

[libvirt] [PATCH 2/2] xenParseXM: don't dereference NULL pointer when script is empty

2011-10-06 Thread Guido Günther 
O.k. to apply?
 -- Guido

---
 src/xenxs/xen_xm.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index d057043..30188e2 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -697,8 +697,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 }
 }
 
-if (bridge[0] || STREQ(script, vif-bridge) ||
-STREQ(script, vif-vnic)) {
+if (bridge[0] || (script  (STREQ(script, vif-bridge) ||
+STREQ(script, vif-vnic {
 net-type = VIR_DOMAIN_NET_TYPE_BRIDGE;
 } else {
 net-type = VIR_DOMAIN_NET_TYPE_ETHERNET;
@@ -715,7 +715,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 !(net-data.bridge.ipaddr = strdup(ip)))
 goto no_memory;
 } else {
-if (script[0] 
+if (script  script[0] 
 !(net-data.ethernet.script = strdup(script)))
 goto no_memory;
 if (ip[0] 
-- 
1.7.6.3

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

[libvirt] [PATCH 1/2] xen: add error handling to UUID parsing

2011-10-06 Thread Guido Günther 
otherwise a missing UUID in a domain config just shows:

libxlDomainXMLFromNative:2600 : Internal Error parsing xm config failed

Now we have:

xenXMConfigGetUUID:186 : Internal Error config value uuid was missing

O.k. to apply?
 -- Guido

---
 src/xenxs/xen_xm.c |   37 +++--
 1 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 03857c8..d057043 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -174,21 +174,38 @@ static int xenXMConfigCopyStringOpt(virConfPtr conf,
 /* Convenience method to grab a string UUID from the config file object */
 static int xenXMConfigGetUUID(virConfPtr conf, const char *name, unsigned char 
*uuid) {
 virConfValuePtr val;
-if (!uuid || !name || !conf)
-return (-1);
+
+if (!uuid || !name || !conf) {
+XENXS_ERROR(VIR_ERR_INVALID_ARG,
+   _(Arguments must be non null));
+return -1;
+}
+
 if (!(val = virConfGetValue(conf, name))) {
-return (-1);
+XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
+   _(config value %s was missing), name);
+return -1;
 }
 
-if (val-type != VIR_CONF_STRING)
-return (-1);
-if (!val-str)
-return (-1);
+if (val-type != VIR_CONF_STRING) {
+XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
+   _(config value %s not a string), name);
+return -1;
+}
+
+if (!val-str) {
+XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
+   _(%s can't be empty), name);
+return -1;
+}
 
-if (virUUIDParse(val-str, uuid)  0)
-return (-1);
+if (virUUIDParse(val-str, uuid)  0) {
+XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
+   _(%s not parseable), val-str);
+return -1;
+}
 
-return (0);
+return 0;
 }
 
 #define MAX_VFB 1024
-- 
1.7.6.3

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

[libvirt] [PATCH] Fix deadlock when the RPC program is unknown

2011-10-06 Thread Daniel P. Berrange 
From: Daniel P. Berrange berra...@redhat.com

Commit 597fe3cee68f561a181967b59a87b4e5c5880c4c accidentally
introduced a deadlock when reporting an unknown RPC program.
The virNetServerDispatchNewMessage method is called with
the client locked, and must therefore not attempt to send
any RPC messages back to the client. Only once the incoming
message is passed off to the virNetServerHandleJob worker
is it safe to start sending messages back

* src/rpc/virnetserver.c: Delay checking for unknown RPC
  program until in worker thread
---
 src/rpc/virnetserver.c |   22 ++
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index d71ed18..9588077 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -133,6 +133,14 @@ static void virNetServerHandleJob(void *jobOpaque, void 
*opaque)
 VIR_DEBUG(server=%p client=%p message=%p prog=%p,
   srv, job-client, job-msg, job-prog);
 
+if (!job-prog) {
+if (virNetServerProgramUnknownError(job-client,
+job-msg,
+job-msg-header)  0)
+goto error;
+goto cleanup;
+}
+
 if (virNetServerProgramDispatch(job-prog,
 srv,
 job-client,
@@ -142,6 +150,8 @@ static void virNetServerHandleJob(void *jobOpaque, void 
*opaque)
 virNetServerLock(srv);
 virNetServerProgramFree(job-prog);
 virNetServerUnlock(srv);
+
+cleanup:
 virNetServerClientFree(job-client);
 VIR_FREE(job);
 return;
@@ -184,18 +194,14 @@ static int 
virNetServerDispatchNewMessage(virNetServerClientPtr client,
 }
 }
 
-if (!prog) {
-virNetServerProgramUnknownError(client, msg, msg-header);
-goto cleanup;
+if (prog) {
+virNetServerProgramRef(prog);
+job-prog = prog;
+priority = virNetServerProgramGetPriority(prog, msg-header.proc);
 }
 
-virNetServerProgramRef(prog);
-job-prog = prog;
-priority = virNetServerProgramGetPriority(prog, msg-header.proc);
-
 ret = virThreadPoolSendJob(srv-workers, priority, job);
 
-cleanup:
 if (ret  0) {
 VIR_FREE(job);
 virNetServerProgramFree(prog);
-- 
1.7.6.2

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

Re: [libvirt] [PATCH 2/2] xenParseXM: don't dereference NULL pointer when script is empty

2011-10-06 Thread Michal Privoznik 
On 06.10.2011 11:16, Guido Günther wrote:
 O.k. to apply?
  -- Guido
 
 ---
  src/xenxs/xen_xm.c |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index d057043..30188e2 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -697,8 +697,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  }
  }
  
 -if (bridge[0] || STREQ(script, vif-bridge) ||
 -STREQ(script, vif-vnic)) {
 +if (bridge[0] || (script  (STREQ(script, vif-bridge) ||

I'd rather use STREQ_NULLABLE here.

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

[libvirt] [PATCH] Don't send back unknown program errors for async messages

2011-10-06 Thread Daniel P. Berrange 
From: Daniel P. Berrange berra...@redhat.com

If we send back an unknown program error for async messages,
we will confuse the client because they only expect replies
for method calls. Just log  drop any invalid async messages

* src/rpc/virnetserver.c: Don't send error for async messages
---
 src/rpc/virnetserver.c|   23 +++
 src/rpc/virnetserverprogram.c |   13 -
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 9588077..f739743 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -134,10 +134,25 @@ static void virNetServerHandleJob(void *jobOpaque, void 
*opaque)
   srv, job-client, job-msg, job-prog);
 
 if (!job-prog) {
-if (virNetServerProgramUnknownError(job-client,
-job-msg,
-job-msg-header)  0)
-goto error;
+/* Only send back an error for type == CALL. Other
+ * message types are not expecting replies, so we
+ * must just log it  drop them
+ */
+if (job-msg-header.type == VIR_NET_CALL) {
+if (virNetServerProgramUnknownError(job-client,
+job-msg,
+job-msg-header)  0)
+goto error;
+} else {
+VIR_INFO(Dropping client mesage, unknown program %d version %d 
type %d proc %d,
+ job-msg-header.prog, job-msg-header.vers,
+ job-msg-header.type, job-msg-header.proc);
+/* Send a dummy reply to free up 'msg'  unblock client rx */
+virNetMessageClear(job-msg);
+job-msg-header.type = VIR_NET_REPLY;
+if (virNetServerClientSendMessage(job-client, job-msg)  0)
+goto error;
+}
 goto cleanup;
 }
 
diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c
index 334a0bf..47b7ded 100644
--- a/src/rpc/virnetserverprogram.c
+++ b/src/rpc/virnetserverprogram.c
@@ -314,7 +314,18 @@ int virNetServerProgramDispatch(virNetServerProgramPtr 
prog,
 return ret;
 
 error:
-ret = virNetServerProgramSendReplyError(prog, client, msg, rerr, 
msg-header);
+if (msg-header.type == VIR_NET_CALL) {
+ret = virNetServerProgramSendReplyError(prog, client, msg, rerr, 
msg-header);
+} else {
+/* Send a dummy reply to free up 'msg'  unblock client rx */
+virNetMessageClear(msg);
+msg-header.type = VIR_NET_REPLY;
+if (virNetServerClientSendMessage(client, msg)  0) {
+ret = -1;
+goto cleanup;
+}
+ret = 0;
+}
 
 cleanup:
 return ret;
-- 
1.7.6.2

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

[libvirt] [PATCH] Make saving domain XML more robust

2011-10-06 Thread Jiri Denemark 
When saving domain XML (either config or status) we just overwrite old
content of the file. In case something fails during that process (e.g.
disk gets full) we lose both old and new content. This patch makes the
process more robust by writing the new content into a separate file and
only if that succeeds the original file is atomically replaced with the
new one.
---
 src/conf/domain_conf.c |   25 +
 1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f9007ce..04c2b1c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11025,6 +11025,7 @@ int virDomainSaveXML(const char *configDir,
  const char *xml)
 {
 char *configFile = NULL;
+char *newfile = NULL;
 int fd = -1, ret = -1;
 size_t towrite;
 
@@ -11038,12 +11039,17 @@ int virDomainSaveXML(const char *configDir,
 goto cleanup;
 }
 
-if ((fd = open(configFile,
+if (virAsprintf(newfile, %s.new, configFile)  0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if ((fd = open(newfile,
O_WRONLY | O_CREAT | O_TRUNC,
S_IRUSR | S_IWUSR ))  0) {
 virReportSystemError(errno,
  _(cannot create config file '%s'),
- configFile);
+ newfile);
 goto cleanup;
 }
 
@@ -11053,14 +11059,21 @@ int virDomainSaveXML(const char *configDir,
 if (safewrite(fd, xml, towrite)  0) {
 virReportSystemError(errno,
  _(cannot write config file '%s'),
- configFile);
+ newfile);
 goto cleanup;
 }
 
 if (VIR_CLOSE(fd)  0) {
 virReportSystemError(errno,
  _(cannot save config file '%s'),
- configFile);
+ newfile);
+goto cleanup;
+}
+
+if (rename(newfile, configFile)  0) {
+virReportSystemError(errno,
+ _(cannot rename config file '%s' as '%s'),
+ newfile, configFile);
 goto cleanup;
 }
 
@@ -11068,6 +11081,10 @@ int virDomainSaveXML(const char *configDir,
  cleanup:
 VIR_FORCE_CLOSE(fd);
 
+if (newfile) {
+unlink(newfile);
+VIR_FREE(newfile);
+}
 VIR_FREE(configFile);
 return ret;
 }
-- 
1.7.7

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

Re: [libvirt] [PATCH] Make saving domain XML more robust

2011-10-06 Thread Daniel P. Berrange 
On Thu, Oct 06, 2011 at 12:22:01PM +0200, Jiri Denemark wrote:
 When saving domain XML (either config or status) we just overwrite old
 content of the file. In case something fails during that process (e.g.
 disk gets full) we lose both old and new content. This patch makes the
 process more robust by writing the new content into a separate file and
 only if that succeeds the original file is atomically replaced with the
 new one.
 ---
  src/conf/domain_conf.c |   25 +
  1 files changed, 21 insertions(+), 4 deletions(-)

By doing this you've almost addressed this BZ...

   https://bugzilla.redhat.com/show_bug.cgi?id=489946

...almost...

Checkout slides 78 - 119 of the presentation linked there,
paying note to the horrible tales of woe about OS-X and fsync() :-(
We probably want to add some kind of virFileSync(int fd)  API call
that does the neccessary black magic.

  
  if (VIR_CLOSE(fd)  0) {
  virReportSystemError(errno,
   _(cannot save config file '%s'),
 - configFile);
 + newfile);
 +goto cleanup;
 +}
 +
 +if (rename(newfile, configFile)  0) {
 +virReportSystemError(errno,
 + _(cannot rename config file '%s' as '%s'),
 + newfile, configFile);
  goto cleanup;
  }

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH 2/2] xenParseXM: don't dereference NULL pointer when script is empty

2011-10-06 Thread Guido Günther 
On Thu, Oct 06, 2011 at 11:56:29AM +0200, Michal Privoznik wrote:
 On 06.10.2011 11:16, Guido Günther wrote:
  O.k. to apply?
   -- Guido
  
  ---
   src/xenxs/xen_xm.c |6 +++---
   1 files changed, 3 insertions(+), 3 deletions(-)
  
  diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
  index d057043..30188e2 100644
  --- a/src/xenxs/xen_xm.c
  +++ b/src/xenxs/xen_xm.c
  @@ -697,8 +697,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
   }
   }
   
  -if (bridge[0] || STREQ(script, vif-bridge) ||
  -STREQ(script, vif-vnic)) {
  +if (bridge[0] || (script  (STREQ(script, vif-bridge) ||
 
 I'd rather use STREQ_NULLABLE here.

In fact I was looking for that kind of function in hacking.html. New
patch attached.
Cheers,
 -- Guido
From 1819f18fbbf0c133a66c93debc117456e940b2bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Guido=20G=C3=BCnther?= a...@sigxcpu.org
Date: Thu, 6 Oct 2011 12:56:52 +0200
Subject: [PATCH] xenParseXM: don't dereference NULL pointer when script is
 empty

---
 src/xenxs/xen_xm.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index d057043..6113f33 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -697,8 +697,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 }
 }
 
-if (bridge[0] || STREQ(script, vif-bridge) ||
-STREQ(script, vif-vnic)) {
+if (bridge[0] || STREQ_NULLABLE(script, vif-bridge) ||
+STREQ_NULLABLE(script, vif-vnic)) {
 net-type = VIR_DOMAIN_NET_TYPE_BRIDGE;
 } else {
 net-type = VIR_DOMAIN_NET_TYPE_ETHERNET;
@@ -715,7 +715,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 !(net-data.bridge.ipaddr = strdup(ip)))
 goto no_memory;
 } else {
-if (script[0] 
+if (script  script[0] 
 !(net-data.ethernet.script = strdup(script)))
 goto no_memory;
 if (ip[0] 
-- 
1.7.6.3

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

[libvirt] [PATCH] Document STR(N)EQ_NULLABLE (was Re: [PATCH 2/2] xenParseXM: don't dereference NULL pointer when script is empty)

2011-10-06 Thread Guido Günther 
On Thu, Oct 06, 2011 at 11:56:29AM +0200, Michal Privoznik wrote:
 On 06.10.2011 11:16, Guido Günther wrote:
  O.k. to apply?
   -- Guido
  
  ---
   src/xenxs/xen_xm.c |6 +++---
   1 files changed, 3 insertions(+), 3 deletions(-)
  
  diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
  index d057043..30188e2 100644
  --- a/src/xenxs/xen_xm.c
  +++ b/src/xenxs/xen_xm.c
  @@ -697,8 +697,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
   }
   }
   
  -if (bridge[0] || STREQ(script, vif-bridge) ||
  -STREQ(script, vif-vnic)) {
  +if (bridge[0] || (script  (STREQ(script, vif-bridge) ||
 
 I'd rather use STREQ_NULLABLE here.
...and here's the doc update.
 -- Guido
From 07b8940e3bb64e2208b191d890e95f059a7ac7a9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Guido=20G=C3=BCnther?= a...@sigxcpu.org
Date: Thu, 6 Oct 2011 13:32:49 +0200
Subject: [PATCH] Document STREQ_NULLABLE and STRNEQ_NULLABLE

---
 docs/hacking.html.in |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 1a32d07..89f9980 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -587,6 +587,12 @@
   STRPREFIX(a,b)
 /pre
   /li
+  lipTo avoid having to check if a or b are NULL:/p
+pre
+  STREQ_NULLABLE(a, b)
+  STRNEQ_NULLABLE(a, b)
+/pre
+  /li
 /ul
 
 
-- 
1.7.6.3

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

[libvirt] [PATCH] Fail gracefully when hashtables are NULL

2011-10-06 Thread Marc-André Lureau 
Instead of the message:
GLib-CRITICAL **: g_hash_table_get_values: assertion `hash_table !=
NULL' failed
---
 libvirt-gobject/libvirt-gobject-connection.c |   18 --
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
b/libvirt-gobject/libvirt-gobject-connection.c
index 95cd878..b7c0178 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -952,11 +952,15 @@ static void gvir_domain_ref(gpointer obj, gpointer ignore 
G_GNUC_UNUSED)
 GList *gvir_connection_get_domains(GVirConnection *conn)
 {
 GVirConnectionPrivate *priv = conn-priv;
-GList *domains;
+GList *domains = NULL;
+
 g_mutex_lock(priv-lock);
-domains = g_hash_table_get_values(priv-domains);
-g_list_foreach(domains, gvir_domain_ref, NULL);
+if (priv-domains != NULL) {
+domains = g_hash_table_get_values(priv-domains);
+g_list_foreach(domains, gvir_domain_ref, NULL);
+}
 g_mutex_unlock(priv-lock);
+
 return domains;
 }
 
@@ -969,11 +973,13 @@ GList *gvir_connection_get_domains(GVirConnection *conn)
 GList *gvir_connection_get_storage_pools(GVirConnection *conn)
 {
 GVirConnectionPrivate *priv = conn-priv;
-GList *pools;
+GList *pools = NULL;
 
 g_mutex_lock(priv-lock);
-pools = g_hash_table_get_values(priv-pools);
-g_list_foreach(pools, gvir_domain_ref, NULL);
+if (priv-pools != NULL) {
+pools = g_hash_table_get_values(priv-pools);
+g_list_foreach(pools, gvir_domain_ref, NULL);
+}
 g_mutex_unlock(priv-lock);
 
 return pools;
-- 
1.7.6.2

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

Re: [libvirt] [PATCH v2 03/12] Introduce two public APIs for keepalive protocol

2011-10-06 Thread Jiri Denemark 
On Mon, Oct 03, 2011 at 10:42:30 +0100, Daniel P. Berrange wrote:
 On Mon, Oct 03, 2011 at 10:26:30AM +0100, Daniel P. Berrange wrote:
  On Fri, Sep 23, 2011 at 10:24:48AM +0200, Jiri Denemark wrote:
   This introduces virConnectAllowKeepAlive and virConnectStartKeepAlive
   public APIs which can be used by a client connecting to remote server to
   indicate support for keepalive protocol. Both APIs are handled directly
   by remote driver and not transmitted over the wire to the server.
   ---
include/libvirt/libvirt.h.in |5 ++
src/driver.h |9 
src/libvirt.c|  107 
   ++
src/libvirt_internal.h   |   10 +++-
src/libvirt_public.syms  |6 ++
5 files changed, 135 insertions(+), 2 deletions(-)
   
   diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
   index 39155a6..6f61cc0 100644
   --- a/include/libvirt/libvirt.h.in
   +++ b/include/libvirt/libvirt.h.in
   @@ -3210,6 +3210,11 @@ typedef struct _virTypedParameter 
   virMemoryParameter;
 */
typedef virMemoryParameter *virMemoryParameterPtr;

   +int virConnectAllowKeepAlive(virConnectPtr conn);
   +int virConnectStartKeepAlive(virConnectPtr conn,
   + int interval,
   + unsigned int count);
  
  With this design, even if both the client and server support
  keepalive at a protocol level, it will only ever be enabled if
  the client application remembers to call virConnectAllowKeepAlive.
  I think this puts too much responsibility on the client, at the
  detriment of the server.
  
  An administrator of libvirtd might want to mandate use of keep
  alive for all clients (knowing this would exclude any libvirt
  client = 0.9.6 of course). With this design they cannot do this
  since they are reliant on the client application programmer to
  call virConnectAllowKeepAlive, which I believe 95% of people will
  never bother todo.
  
  IMHO we should change this so that
  
   - In remote_driver.c, doRemoteOpen(), after performing
 authentication, but before opening the connection
 we should send a
  
supports_feature(KEEPALIVE)
  
   - Upon receiving supports_feature(KEEPALIVE) the server
 shall be free to start sending keep alives, if it is
 configured todo so.
 
 
 Hmm, actually I see elsewhere that responding to server initiated
 pings, requires help of the event loop which we can't assume is
 running.
 
 I guess we could enable it if we detect that an event loop has been
 registered.  This would make us hit the virsh bug again where we
 register the event loop, but never run it. Arguably that bug should
 be fixed in virsh for other reasons, so perhaps this is justification
 enough todo so.

Right, we could do that once the bug in virsh is fixed so I'll try to
incorporate the virsh fix into this series.

 If only we had made use of an event loop mandatory all those years
 ago :-(

Oh yes, client-side implementation would be much more simple without the buck
passing algorithm. Although that would mean significantly less fun on that
part :-)

Jirka

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

Re: [libvirt] virsh bash completion file

2011-10-06 Thread Serge E. Hallyn 
Quoting Daniel P. Berrange (berra...@redhat.com):
 On Wed, Oct 05, 2011 at 03:17:47PM -0500, Serge E. Hallyn wrote:
  Hi,
  
  I've been trying out a bash autocompletion file by Geoff Low (slight hack
  by me, don't blame him for my hack), and it's working pretty nicely.
  I'm not sure where to put it in the git tree, but it seems like it'd be
  nice to have upstream?
 
 David Lutterkort previously suggested this
 
https://www.redhat.com/archives/libvir-list/2007-October/msg00231.html
 
 And I didn't ever notice that and so wrote one myself
 
https://www.redhat.com/archives/libvir-list/2008-July/msg00175.html
https://www.redhat.com/archives/libvir-list/2008-July/msg00177.html
 
 Neither ever got committed. There were some things I didn't much like
 about mine, but I can't really remember now.
 
 Third time lucky perhaps :-)

So the things you didn't much like must not have been *too* bad?  :)
Are you going to make some changes and resubmit, or would you like
me to test (and port as needed) the above patches first?

thanks,
-serge

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

[libvirt] [PATCH] remote_driver: Avoid double free in EventControl building

2011-10-06 Thread Michal Privoznik 
Don't xdr_free event data as they are freed by our caller
virNetClientProgramDispatch.
---
 src/remote/remote_driver.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 83f4f3c..2b2f41e 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3315,7 +3315,6 @@ remoteDomainBuildEventControlError(virNetClientProgramPtr 
prog ATTRIBUTE_UNUSED,
 return;
 
 event = virDomainEventControlErrorNewFromDom(dom);
-xdr_free ((xdrproc_t) xdr_remote_domain_event_control_error_msg, (char *) 
msg);
 
 virDomainFree(dom);
 
-- 
1.7.3.4

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

Re: [libvirt] [PATCH] Fix deadlock when the RPC program is unknown

2011-10-06 Thread Michal Privoznik 
On 06.10.2011 11:43, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Commit 597fe3cee68f561a181967b59a87b4e5c5880c4c accidentally
 introduced a deadlock when reporting an unknown RPC program.
 The virNetServerDispatchNewMessage method is called with
 the client locked, and must therefore not attempt to send
 any RPC messages back to the client. Only once the incoming
 message is passed off to the virNetServerHandleJob worker
 is it safe to start sending messages back
 
 * src/rpc/virnetserver.c: Delay checking for unknown RPC
   program until in worker thread
 ---
  src/rpc/virnetserver.c |   22 ++
  1 files changed, 14 insertions(+), 8 deletions(-)
 

ACK

Michal

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

Re: [libvirt] [PATCH] Don't send back unknown program errors for async messages

2011-10-06 Thread Eric Blake 

On 10/06/2011 04:10 AM, Daniel P. Berrange wrote:

From: Daniel P. Berrangeberra...@redhat.com

If we send back an unknown program error for async messages,
we will confuse the client because they only expect replies
for method calls. Just log  drop any invalid async messages

* src/rpc/virnetserver.c: Don't send error for async messages
---
  src/rpc/virnetserver.c|   23 +++
  src/rpc/virnetserverprogram.c |   13 -
  2 files changed, 31 insertions(+), 5 deletions(-)


ACK.


+} else {
+VIR_INFO(Dropping client mesage, unknown program %d version %d type %d 
proc %d,


s/mesage/message/

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] Document STR(N)EQ_NULLABLE (was Re: [PATCH 2/2] xenParseXM: don't dereference NULL pointer when script is empty)

2011-10-06 Thread Eric Blake 

On 10/06/2011 05:38 AM, Guido Günther wrote:

...and here's the doc update.
  -- Guido


0001-Document-STREQ_NULLABLE-and-STRNEQ_NULLABLE.patch



From 07b8940e3bb64e2208b191d890e95f059a7ac7a9 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Guido=20G=C3=BCnther?=a...@sigxcpu.org
Date: Thu, 6 Oct 2011 13:32:49 +0200
Subject: [PATCH] Document STREQ_NULLABLE and STRNEQ_NULLABLE

---
  docs/hacking.html.in |6 ++
  1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 1a32d07..89f9980 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -587,6 +587,12 @@
STRPREFIX(a,b)
  /pre
/li
+lipTo avoid having to check if a or b are NULL:/p
+pre
+  STREQ_NULLABLE(a, b)
+  STRNEQ_NULLABLE(a, b)
+/pre
+/li


ACK.  But be sure you also run 'make check', then squash in the 
regeneration of HACKING prior to pushing your commit.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] Document STR(N)EQ_NULLABLE (was Re: [PATCH 2/2] xenParseXM: don't dereference NULL pointer when script is empty)

2011-10-06 Thread Guido Günther 
On Thu, Oct 06, 2011 at 08:27:42AM -0600, Eric Blake wrote:
 On 10/06/2011 05:38 AM, Guido Günther wrote:
 ...and here's the doc update.
   -- Guido
 
 
 0001-Document-STREQ_NULLABLE-and-STRNEQ_NULLABLE.patch
 
 
 From 07b8940e3bb64e2208b191d890e95f059a7ac7a9 Mon Sep 17 00:00:00 2001
 From: =?UTF-8?q?Guido=20G=C3=BCnther?=a...@sigxcpu.org
 Date: Thu, 6 Oct 2011 13:32:49 +0200
 Subject: [PATCH] Document STREQ_NULLABLE and STRNEQ_NULLABLE
 
 ---
   docs/hacking.html.in |6 ++
   1 files changed, 6 insertions(+), 0 deletions(-)
 
 diff --git a/docs/hacking.html.in b/docs/hacking.html.in
 index 1a32d07..89f9980 100644
 --- a/docs/hacking.html.in
 +++ b/docs/hacking.html.in
 @@ -587,6 +587,12 @@
 STRPREFIX(a,b)
   /pre
 /li
 +lipTo avoid having to check if a or b are NULL:/p
 +pre
 +  STREQ_NULLABLE(a, b)
 +  STRNEQ_NULLABLE(a, b)
 +/pre
 +/li
 
 ACK.  But be sure you also run 'make check', then squash in the
 regeneration of HACKING prior to pushing your commit.

I had to do a explicit make HACKING to get this regenerated. Pushed
now - thanks.
Cheers,
 -- Guido

 
 -- 
 Eric Blake   ebl...@redhat.com+1-801-349-2682
 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] Document STR(N)EQ_NULLABLE (was Re: [PATCH 2/2] xenParseXM: don't dereference NULL pointer when script is empty)

2011-10-06 Thread Eric Blake 

On 10/06/2011 08:52 AM, Guido Günther wrote:

ACK.  But be sure you also run 'make check', then squash in the
regeneration of HACKING prior to pushing your commit.


I had to do a explicit make HACKING to get this regenerated. Pushed
now - thanks.


Ah, maybe it was only 'make syntax-check' that does the auto-regen.  At 
any rate, thank you for the patch.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] [libvirt-glib] API to deal with storage pool(s)

2011-10-06 Thread Christophe Fergeau 
On Tue, Sep 27, 2011 at 01:19:56AM +0300, Zeeshan Ali (Khattak) wrote:
 +gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn,
 + GCancellable *cancellable,
 + GError **err)
 +{
 +GVirConnectionPrivate *priv = conn-priv;
 +GHashTable *pools;
 +gchar **inactive = NULL;
 +gint ninactive = 0;
 +gchar **active = NULL;
 +gint nactive = 0;
 +gboolean ret = FALSE;
 +gint i;
 +virConnectPtr vconn = NULL;
 +
 +g_mutex_lock(priv-lock);
 +if (!priv-conn) {
 +*err = gvir_error_new(GVIR_CONNECTION_ERROR,
 +  0,
 +  Connection is not open);

gvir_error_new creates a new GError and automatically appends the last
error message reported by libvirt (if any) to it. In this case, won't we
get a potentially confusing error message from libvirt since we don't
really know what happened before this function was called? The same pattern
occurs several times throughout this file.

Christophe


pgpjyRFWUmFX5.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: enable multifunction for older qemu

2011-10-06 Thread Eric Blake 

On 10/05/2011 09:10 PM, Wen Congyang wrote:

At 10/06/2011 05:52 AM, Eric Blake Write:

Now that RHEL 6.2 Beta is out, it would be nice to test multifunction
devices on that platform.  This changes things so that the multifunction
cap bit can be set in two different ways: by version comparison (needed
for qemu 0.13 which lacked a -device query), and by -device query
(provided by qemu.git and backported to the RHEL beta build of
qemu-kvm which still claims to be a modified 0.12, and therefore needed
for RHEL).




+++ b/src/qemu/qemu_capabilities.c
@@ -1264,6 +1264,8 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr 
flags)
  /* Features of given devices. */
  if (strstr(str, pci-assign.configfd))
  qemuCapsSet(flags, QEMU_CAPS_PCI_CONFIGFD);
+if (strstr(str, pci-assign.multifunction))
+qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION);


Only qemu-kvm has pci-assign, so I think it is better to check 
'virtio-blk-pci.multifunction'.


Good point.  I've made that adjustment, and pushed.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
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 1/2] xen: add error handling to UUID parsing

2011-10-06 Thread Eric Blake 

On 10/06/2011 03:15 AM, Guido Günther wrote:

otherwise a missing UUID in a domain config just shows:

libxlDomainXMLFromNative:2600 : Internal Error parsing xm config failed

Now we have:

xenXMConfigGetUUID:186 : Internal Error config value uuid was missing


Still doesn't sound quite right - it's not an internal error (where 
libvirt has gotten internal logic wrong) so much as a user-supplied data 
validation error.



+if (val-type != VIR_CONF_STRING) {
+XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
+   _(config value %s not a string), name);
+return -1;
+}
+
+if (!val-str) {
+XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
+   _(%s can't be empty), name);
+return -1;
+}

-if (virUUIDParse(val-str, uuid)  0)
-return (-1);
+if (virUUIDParse(val-str, uuid)  0) {
+XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
+   _(%s not parseable), val-str);


These three errors should probably all be changed away from 
VIR_ERR_INTERNAL_ERROR into something more useful, but I'm not sure 
whether that would be VIR_ERR_CONF_SYNTAX, VIR_ERR_CONFIG_UNSUPPORTED, 
or something else.


ACK to the concept, once we decide on the correct error code.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
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 1.5/2] qemu: leave rerror policy at default when enospace is requested

2011-10-06 Thread Eric Blake 

On 10/05/2011 09:06 PM, Laine Stump wrote:

commit 12062ab set rerror=ignore when error_policy=enospace was
selected (since the rerror option in qemu doesn't accept enospc, as
the werror option does).

After that patch was already pushed, Paolo Bonzini noticed it and
commented that leaving rerror at the default (report) would be a
better choice. This patch corrects the problem - if error_policy =
enospace is given, rerror is left off the qemu commandline,
effectively setting it to report. For other values, rerror is still
set to match werror.

Additionally, the parsing of error_policy was changed to no longer
erroneously allow default as a choice - as with most other
attributes, if you want the default setting, just don't specify an
error_policy.

Finally, two ommissions in the first patch were corrected - a


s/ommissions/omissions/


long-dormant qemuxml2argv test for enospace was enabled, and fixed to
pass, and the argv2xml parser in qemu_command.c was updated to
recognize the different spelling on the qemu commandline.
---
  src/conf/domain_conf.c |2 +-
  src/qemu/qemu_command.c|   15 ---
  ...uxml2argv-disk-drive-error-policy-enospace.args |2 +-
  tests/qemuxml2argvtest.c   |2 ++
  4 files changed, 12 insertions(+), 9 deletions(-)



ACK.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] [PATCHv2 2/2] qemu: add separate rerror_policy for disk errors

2011-10-06 Thread Eric Blake 

On 10/05/2011 09:06 PM, Laine Stump wrote:

Previously libvirt's disk device XML only had a single attribute,
error_policy, to control both read and write error policy, but qemu
has separate options for controlling read and write. In one case
(enospc) a policy is allowed for write errors but not read errors.

This patch adds a separate attribute that sets only the read error
policy. If just error_policy is set, it will apply to both read and
write error policy (previous behavior), but if the new rerror_policy
attribute is set, it will override error_policy for read errors only.
Possible values for rerror_policy are stop, report, and ignore
(report is the qemu-controlled default for rerror_policy when
error_policy isn't specified).

For consistency, the value report has been added to the possible
values for error_policy as well.


ACK.  This turned out a lot nicer compared to v1, now that we had time 
to think about the issues.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
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 1/2] xen: add error handling to UUID parsing

2011-10-06 Thread Guido Günther 
Hi Eric,
On Thu, Oct 06, 2011 at 10:50:46AM -0600, Eric Blake wrote:
 On 10/06/2011 03:15 AM, Guido Günther wrote:
 otherwise a missing UUID in a domain config just shows:
 
 libxlDomainXMLFromNative:2600 : Internal Error parsing xm config failed
 
 Now we have:
 
 xenXMConfigGetUUID:186 : Internal Error config value uuid was missing
 
 Still doesn't sound quite right - it's not an internal error (where
 libvirt has gotten internal logic wrong) so much as a user-supplied
 data validation error.
 
 +if (val-type != VIR_CONF_STRING) {
 +XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
 +   _(config value %s not a string), name);
 +return -1;
 +}
 +
 +if (!val-str) {
 +XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
 +   _(%s can't be empty), name);
 +return -1;
 +}
 
 -if (virUUIDParse(val-str, uuid)  0)
 -return (-1);
 +if (virUUIDParse(val-str, uuid)  0) {
 +XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
 +   _(%s not parseable), val-str);
 
 These three errors should probably all be changed away from
 VIR_ERR_INTERNAL_ERROR into something more useful, but I'm not sure
 whether that would be VIR_ERR_CONF_SYNTAX,
 VIR_ERR_CONFIG_UNSUPPORTED, or something else.

I was uncertain about the error codes too, that's why I kept
VIR_ERR_INTERNAL_ERROR as used by other functions in this file. I'd
opt for VIR_ERR_CONF_SYNTAX.
 -- Guido

 
 ACK to the concept, once we decide on the correct error code.
 
 -- 
 Eric Blake   ebl...@redhat.com+1-801-349-2682
 Libvirt virtualization library http://libvirt.org
 

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

Re: [libvirt] [libvirt-glib] API to deal with storage pool(s)

2011-10-06 Thread Daniel P. Berrange 
On Thu, Oct 06, 2011 at 06:46:05PM +0200, Christophe Fergeau wrote:
 On Tue, Sep 27, 2011 at 01:19:56AM +0300, Zeeshan Ali (Khattak) wrote:
  +gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn,
  + GCancellable *cancellable,
  + GError **err)
  +{
  +GVirConnectionPrivate *priv = conn-priv;
  +GHashTable *pools;
  +gchar **inactive = NULL;
  +gint ninactive = 0;
  +gchar **active = NULL;
  +gint nactive = 0;
  +gboolean ret = FALSE;
  +gint i;
  +virConnectPtr vconn = NULL;
  +
  +g_mutex_lock(priv-lock);
  +if (!priv-conn) {
  +*err = gvir_error_new(GVIR_CONNECTION_ERROR,
  +  0,
  +  Connection is not open);
 
 gvir_error_new creates a new GError and automatically appends the last
 error message reported by libvirt (if any) to it. In this case, won't we
 get a potentially confusing error message from libvirt since we don't
 really know what happened before this function was called? The same pattern
 occurs several times throughout this file.

Yep, gvir_error_new should only be used immediately after a libvirt
API call has failed. In any other case, use the normal g_error_new
functions


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

[libvirt] [libvirt-glib 1/3] Don't use empty message when building a GError

2011-10-06 Thread Christophe Fergeau 
After testing that 'message' is NULL, gvir_error_new_literal
is using it to build a GError. What was meant was to use
verr-message.
---
 libvirt-glib/libvirt-glib-error.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libvirt-glib/libvirt-glib-error.c 
b/libvirt-glib/libvirt-glib-error.c
index 7afa802..f59b464 100644
--- a/libvirt-glib/libvirt-glib-error.c
+++ b/libvirt-glib/libvirt-glib-error.c
@@ -92,7 +92,7 @@ GError *gvir_error_new_literal(GQuark domain,
 return g_error_new(domain,
code,
%s,
-   message);
+   verr-message);
 }
 
 /**
-- 
1.7.6.4

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

[libvirt] [libvirt-glib 2/3] Don't use empty message when building a GError

2011-10-06 Thread Christophe Fergeau 
After testing that 'message' is NULL, gvir_xml_error_new_literal
is using it to build a GError. What was meant was to use
xerr-message.
---
 libvirt-gconfig/libvirt-gconfig-object.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libvirt-gconfig/libvirt-gconfig-object.c 
b/libvirt-gconfig/libvirt-gconfig-object.c
index 358c9e1..95b147f 100644
--- a/libvirt-gconfig/libvirt-gconfig-object.c
+++ b/libvirt-gconfig/libvirt-gconfig-object.c
@@ -83,7 +83,7 @@ static GError *gvir_xml_error_new_literal(GQuark domain,
 return g_error_new(domain,
code,
%s,
-   message);
+   xerr-message);
 }
 
 
-- 
1.7.6.4

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

[libvirt] [libvirt-glib 3/3] Only call gvir_error_new after failed libvirt calls

2011-10-06 Thread Christophe Fergeau 
gvir_error_new is only meant to be used right after a failed
libvirt function call, in other cases we should be calling
g_error_new directly.
---
 libvirt-gobject/libvirt-gobject-connection.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
b/libvirt-gobject/libvirt-gobject-connection.c
index 95cd878..70b7411 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -583,9 +583,9 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn,
 
 g_mutex_lock(priv-lock);
 if (!priv-conn) {
-*err = gvir_error_new(GVIR_CONNECTION_ERROR,
-  0,
-  Connection is not open);
+*err = g_error_new(GVIR_CONNECTION_ERROR,
+   0,
+   Connection is not open);
 g_mutex_unlock(priv-lock);
 goto cleanup;
 }
@@ -708,9 +708,9 @@ gboolean gvir_connection_fetch_storage_pools(GVirConnection 
*conn,
 
 g_mutex_lock(priv-lock);
 if (!priv-conn) {
-*err = gvir_error_new(GVIR_CONNECTION_ERROR,
-  0,
-  Connection is not open);
+*err = g_error_new(GVIR_CONNECTION_ERROR,
+   0,
+   Connection is not open);
 g_mutex_unlock(priv-lock);
 goto cleanup;
 }
-- 
1.7.6.4

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

Re: [libvirt] [libvirt-glib] Turn GVirStream into a GIOStream

2011-10-06 Thread Christophe Fergeau 
On Sat, Oct 01, 2011 at 07:57:46PM +0200, Marc-André Lureau wrote:
 Allows to read async from a stream with GVirInputStream.
 This is modelled after GSocket.
  
  
  enum {
 @@ -60,6 +64,71 @@ gvir_stream_error_quark(void)
  return g_quark_from_static_string(vir-g-stream);
  }

I'd rename it to gvir-stream to be consistent with the other quarks defined
in libvirt-glib.

Christophe


pgpZnUroi0ruH.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] xenParseXM: don't dereference NULL pointer when script is empty

2011-10-06 Thread Eric Blake 

On 10/06/2011 05:36 AM, Guido Günther wrote:

In fact I was looking for that kind of function in hacking.html. New
patch attached.
Cheers,
  -- Guido


0001-xenParseXM-don-t-dereference-NULL-pointer-when-scrip.patch



From 1819f18fbbf0c133a66c93debc117456e940b2bc Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Guido=20G=C3=BCnther?=a...@sigxcpu.org
Date: Thu, 6 Oct 2011 12:56:52 +0200
Subject: [PATCH] xenParseXM: don't dereference NULL pointer when script is
  empty

---
  src/xenxs/xen_xm.c |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index d057043..6113f33 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -697,8 +697,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  }
  }

-if (bridge[0] || STREQ(script, vif-bridge) ||
-STREQ(script, vif-vnic)) {
+if (bridge[0] || STREQ_NULLABLE(script, vif-bridge) ||
+STREQ_NULLABLE(script, vif-vnic)) {
  net-type = VIR_DOMAIN_NET_TYPE_BRIDGE;
  } else {
  net-type = VIR_DOMAIN_NET_TYPE_ETHERNET;
@@ -715,7 +715,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  !(net-data.bridge.ipaddr = strdup(ip)))
  goto no_memory;
  } else {
-if (script[0]
+if (script  script[0]
  !(net-data.ethernet.script = strdup(script)))
  goto no_memory;
  if (ip[0]
-- 1.7.6.3


ACK.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] remote_driver: Avoid double free in EventControl building

2011-10-06 Thread Eric Blake 

On 10/06/2011 07:41 AM, Michal Privoznik wrote:

Don't xdr_free event data as they are freed by our caller
virNetClientProgramDispatch.
---
  src/remote/remote_driver.c |1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 83f4f3c..2b2f41e 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3315,7 +3315,6 @@ remoteDomainBuildEventControlError(virNetClientProgramPtr 
prog ATTRIBUTE_UNUSED,
  return;

  event = virDomainEventControlErrorNewFromDom(dom);
-xdr_free ((xdrproc_t)xdr_remote_domain_event_control_error_msg, (char 
*)msg);


ACK - matches that mgs was assigned to evdata passed in as parameter 
rather than allocated here in remoteDomainBuildEventControlError.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
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 2/2] xenParseXM: don't dereference NULL pointer when script is empty

2011-10-06 Thread Guido Günther 
On Thu, Oct 06, 2011 at 12:44:05PM -0600, Eric Blake wrote:
 On 10/06/2011 05:36 AM, Guido Günther wrote:
 In fact I was looking for that kind of function in hacking.html. New
 patch attached.
 Cheers,
   -- Guido
 
 
 0001-xenParseXM-don-t-dereference-NULL-pointer-when-scrip.patch
 
 
 From 1819f18fbbf0c133a66c93debc117456e940b2bc Mon Sep 17 00:00:00 2001
 From: =?UTF-8?q?Guido=20G=C3=BCnther?=a...@sigxcpu.org
 Date: Thu, 6 Oct 2011 12:56:52 +0200
 Subject: [PATCH] xenParseXM: don't dereference NULL pointer when script is
   empty
 
 ---
   src/xenxs/xen_xm.c |6 +++---
   1 files changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index d057043..6113f33 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -697,8 +697,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
   }
   }
 
 -if (bridge[0] || STREQ(script, vif-bridge) ||
 -STREQ(script, vif-vnic)) {
 +if (bridge[0] || STREQ_NULLABLE(script, vif-bridge) ||
 +STREQ_NULLABLE(script, vif-vnic)) {
   net-type = VIR_DOMAIN_NET_TYPE_BRIDGE;
   } else {
   net-type = VIR_DOMAIN_NET_TYPE_ETHERNET;
 @@ -715,7 +715,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
   !(net-data.bridge.ipaddr = strdup(ip)))
   goto no_memory;
   } else {
 -if (script[0]
 +if (script  script[0]
   !(net-data.ethernet.script = strdup(script)))
   goto no_memory;
   if (ip[0]
 -- 1.7.6.3
 
 ACK.
Pushed. Thanks, 
 -- Guido

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

Re: [libvirt] [PATCH] build: fix 'make distcheck' with pdwtags installed

2011-10-06 Thread Laine Stump 

On 10/04/2011 12:33 PM, Eric Blake wrote:

I am getting this failure with 'make distcheck':

   GEN../../src/remote_protocol-structs
/bin/sh: ../../src/remote_protocol-structs-t: Permission denied
make[4]: *** [../../src/remote_protocol-structs] Error 1

since it attempts a sub-run of a VPATH 'make check' where $(srcdir)
is intentionally read-only.  I'm not sure which commit introduced
the problem, although I suspect it was around 62dee6f when I
refactored protocol struct checking to be more powerful.

$(@F) is required by POSIX, and although it is not yet portable
to all make implementations, we already require GNU make.

* src/Makefile.am (PDWTAGS): Generate temp file into current
directory, since $(srcdir) is read-only during distcheck.
---
  src/Makefile.am |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 738ee91..9650139 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -255,9 +255,9 @@ PDWTAGS = \
-e 'exit 8;'\
-e '  }'\
-e '}'  \
- $@-t; \
+ $(@F)-t;  \
  case $$? in 8) exit 0;; 0) ;; *) exit 1;; esac;   \
- diff -u $@-t $@; st=$$?; rm -f $@-t; exit $$st;   \
+ diff -u $(@F)-t $@; st=$$?; rm -f $(@F)-t; exit $$st; \
else\
  echo 'WARNING: you lack pdwtags; skipping the $@ test'2; \
  echo 'WARNING: install the dwarves package to get pdwtags'2; \


ACK.

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

Re: [libvirt] [PATCH 1/2] xen: add error handling to UUID parsing

2011-10-06 Thread Laine Stump 

On 10/06/2011 01:36 PM, Guido Günther wrote:

Hi Eric,
On Thu, Oct 06, 2011 at 10:50:46AM -0600, Eric Blake wrote:


These three errors should probably all be changed away from
VIR_ERR_INTERNAL_ERROR into something more useful, but I'm not sure
whether that would be VIR_ERR_CONF_SYNTAX,
VIR_ERR_CONFIG_UNSUPPORTED, or something else.

I was uncertain about the error codes too, that's why I kept
VIR_ERR_INTERNAL_ERROR as used by other functions in this file. I'd
opt for VIR_ERR_CONF_SYNTAX.


We really need to agree on a policy on which code to use for which type 
of errors, write it down, then go through the code and correct 
everything that's wrong. INTERNAL_ERROR is used *a lot* in XML parsing 
where it obviously shouldn't be. Much of it is probably code written 
before the CONFIG_UNSUPPORTED, XML_ERROR, and CONF_SYNTAX codes were 
added, but I'd wager just as much is due to people writing new code who 
look to the existing code for guidelines (I know I've done this a lot, 
and continue to be confused about which code to use in a lot of cases - 
heck, this is the first I've noticed CONF_SYNTAX, and I'm not sure when 
it would be appropriate to use that vs. XML_ERROR).


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

Re: [libvirt] [PATCH] Document STR(N)EQ_NULLABLE (was Re: [PATCH 2/2] xenParseXM: don't dereference NULL pointer when script is empty)

2011-10-06 Thread Guido Günther 
On Thu, Oct 06, 2011 at 08:54:22AM -0600, Eric Blake wrote:
 On 10/06/2011 08:52 AM, Guido Günther wrote:
 ACK.  But be sure you also run 'make check', then squash in the
 regeneration of HACKING prior to pushing your commit.
 
 I had to do a explicit make HACKING to get this regenerated. Pushed
 now - thanks.
 
 Ah, maybe it was only 'make syntax-check' that does the auto-regen.
 At any rate, thank you for the patch.

I checked and it's make syntax-check.
Thanks,
 -- Guido

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

Re: [libvirt] [PATCH 1/2] xen: add error handling to UUID parsing

2011-10-06 Thread Guido Günther 
On Thu, Oct 06, 2011 at 03:55:48PM -0400, Laine Stump wrote:
 On 10/06/2011 01:36 PM, Guido Günther wrote:
 Hi Eric,
 On Thu, Oct 06, 2011 at 10:50:46AM -0600, Eric Blake wrote:
 
 These three errors should probably all be changed away from
 VIR_ERR_INTERNAL_ERROR into something more useful, but I'm not sure
 whether that would be VIR_ERR_CONF_SYNTAX,
 VIR_ERR_CONFIG_UNSUPPORTED, or something else.
 I was uncertain about the error codes too, that's why I kept
 VIR_ERR_INTERNAL_ERROR as used by other functions in this file. I'd
 opt for VIR_ERR_CONF_SYNTAX.
 
 We really need to agree on a policy on which code to use for which
 type of errors, write it down, then go through the code and correct
 everything that's wrong. INTERNAL_ERROR is used *a lot* in XML
 parsing where it obviously shouldn't be. Much of it is probably code
 written before the CONFIG_UNSUPPORTED, XML_ERROR, and CONF_SYNTAX
 codes were added, but I'd wager just as much is due to people
 writing new code who look to the existing code for guidelines (I
 know I've done this a lot, and continue to be confused about which
 code to use in a lot of cases - heck, this is the first I've noticed
 CONF_SYNTAX, and I'm not sure when it would be appropriate to use
 that vs. XML_ERROR).

We're not parsing XML here but xen configuration so CONF_SYNTAX fits
better. When parsing XML XML_ERROR makes more sense but you're right
that's mostly guessing at the moment.
Cheers,
 -- Guido

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

Re: [libvirt] [PATCH 1/2] xen: add error handling to UUID parsing

2011-10-06 Thread Eric Blake 

On 10/06/2011 11:36 AM, Guido Günther wrote:

These three errors should probably all be changed away from
VIR_ERR_INTERNAL_ERROR into something more useful, but I'm not sure
whether that would be VIR_ERR_CONF_SYNTAX,
VIR_ERR_CONFIG_UNSUPPORTED, or something else.


I was uncertain about the error codes too, that's why I kept
VIR_ERR_INTERNAL_ERROR as used by other functions in this file. I'd
opt for VIR_ERR_CONF_SYNTAX.
  -- Guido


Yeah, for this particular case, where we are parsing a xen conf file, 
CONF_SYNTAX is the best fit.






ACK to the concept, once we decide on the correct error code.


I think we've decided.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

[libvirt] [PATCH 0/2] snapshot: allow editing disk-only snapshots

2011-10-06 Thread Eric Blake 
I tested that both of these patches in isolation were sufficient
to fix 'virsh snapshot-current dom name' when running the same
version of virsh as libvirtd (what will become 0.9.7).  However,
both patches are required, so that both virsh 0.9.5 and libvirtd
0.9.7, as well as virsh 0.9.7 and libvirt 0.9.5, will work (leaving
only the combination of 0.9.5 in both virsh and libvirtd broken).

Eric Blake (2):
  snapshot: let virsh edit disk snapshots
  snapshot: simplify redefinition of disk snapshot

 src/conf/domain_conf.c |   44 +++-
 tools/virsh.c  |5 +
 2 files changed, 28 insertions(+), 21 deletions(-)

-- 
1.7.4.4

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

[libvirt] [PATCH 1/2] snapshot: let virsh edit disk snapshots

2011-10-06 Thread Eric Blake 
It was impossible for 'virsh snapshot-current dom name' to set name
as the current snapshot, if name is a disk-only snapshot.

Using strstr rather than full-blown xml parsing is safe, since the
xml is assumed to be well-formed coming from libvirtd rather than
arbitrary text coming from the user.

* tools/virsh.c (cmdSnapshotCurrent, cmdSnapshotEdit): Pass
disk_only flag when redefining a disk snapshot.
---
 tools/virsh.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 48f2b8a..70a4078 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12869,6 +12869,9 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
 virDomainSnapshotFree(snapshot);
 snapshot = NULL;

+if (strstr(doc, statedisk-snapshot/state))
+define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
+
 /* Create and open the temporary file.  */
 tmp = editWriteToTempFile(ctl, doc);
 if (!tmp)
@@ -12978,6 +12981,8 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
 xml = virDomainSnapshotGetXMLDesc(snapshot, VIR_DOMAIN_XML_SECURE);
 if (!xml)
 goto cleanup;
+if (strstr(xml, statedisk-snapshot/state))
+flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
 snapshot2 = virDomainSnapshotCreateXML(dom, xml, flags);
 if (snapshot2 == NULL)
 goto cleanup;
-- 
1.7.4.4

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

[libvirt] [PATCH 2/2] snapshot: simplify redefinition of disk snapshot

2011-10-06 Thread Eric Blake 
Redefining disk-only snapshot xml should work even if the user
did not explicitly pass VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
the flag is only required for conditions where the state
subelement is not already present in parsing (that is, defining
a new snapshot).

Also, fix the error code of some user-visible errors (the remaining
VIR_ERR_INTERNAL_ERROR should not be user-visible, since parsing
of active is only done from internal code).

* src/conf/domain_conf.c (virDomainSnapshotDefParseString): Allow
disks during redefinition of disk snapshot.
---
 src/conf/domain_conf.c |   44 +++-
 1 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f9007ce..34bf2d0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11696,25 +11696,6 @@ virDomainSnapshotDefParseString(const char *xmlStr,

 def-description = virXPathString(string(./description), ctxt);

-if ((i = virXPathNodeSet(./disks/*, ctxt, nodes))  0)
-goto cleanup;
-if (flags  VIR_DOMAIN_SNAPSHOT_PARSE_DISKS) {
-def-ndisks = i;
-if (def-ndisks  VIR_ALLOC_N(def-disks, def-ndisks)  0) {
-virReportOOMError();
-goto cleanup;
-}
-for (i = 0; i  def-ndisks; i++) {
-if (virDomainSnapshotDiskDefParseXML(nodes[i], def-disks[i])  0)
-goto cleanup;
-}
-VIR_FREE(nodes);
-} else if (i) {
-virDomainReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
- _(unable to handle disk requests in snapshot));
-goto cleanup;
-}
-
 if (flags  VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
 if (virXPathLongLong(string(./creationTime), ctxt,
  def-creationTime)  0) {
@@ -11730,13 +11711,13 @@ virDomainSnapshotDefParseString(const char *xmlStr,
 /* there was no state in an existing snapshot; this
  * should never happen
  */
-virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s,
+virDomainReportError(VIR_ERR_XML_ERROR, %s,
  _(missing state from existing snapshot));
 goto cleanup;
 }
 def-state = virDomainSnapshotStateTypeFromString(state);
 if (def-state  0) {
-virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+virDomainReportError(VIR_ERR_XML_ERROR,
  _(Invalid state '%s' in domain snapshot 
XML),
  state);
 goto cleanup;
@@ -11768,6 +11749,27 @@ virDomainSnapshotDefParseString(const char *xmlStr,
 def-creationTime = tv.tv_sec;
 }

+if ((i = virXPathNodeSet(./disks/*, ctxt, nodes))  0)
+goto cleanup;
+if (flags  VIR_DOMAIN_SNAPSHOT_PARSE_DISKS ||
+(flags  VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE 
+ def-state == VIR_DOMAIN_DISK_SNAPSHOT)) {
+def-ndisks = i;
+if (def-ndisks  VIR_ALLOC_N(def-disks, def-ndisks)  0) {
+virReportOOMError();
+goto cleanup;
+}
+for (i = 0; i  def-ndisks; i++) {
+if (virDomainSnapshotDiskDefParseXML(nodes[i], def-disks[i])  0)
+goto cleanup;
+}
+VIR_FREE(nodes);
+} else if (i) {
+virDomainReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+ _(unable to handle disk requests in snapshot));
+goto cleanup;
+}
+
 if (flags  VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) {
 if (virXPathInt(string(./active), ctxt, active)  0) {
 virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s,
-- 
1.7.4.4

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

[libvirt] [PATCH 14/7] snapshot: virsh shorthand for operating on current snap

2011-10-06 Thread Eric Blake 
Rather than having to do:

$ virsh snapshot-revert dom $(virsh snapshot-current dom --name)

I thought it would be nice to do:

$ virsh snaphot-revert dom --current

I intentionally made it so that omitting both --current and a name
is an error (having the absence of a name imply --current seems
just a bit too magic, so --current must be explicit).

I didn't add 'virsh snapshot-dumpxml --current' since we already
have 'virsh snapshot-current' for the same task.  All other
snapshot-* options that take a snapshotname now take --current in
its place; while keeping snapshot-edit backwards-compatible.

* tools/virsh.c (vshLookupSnapshot): New helper function.
(cmdSnapshotEdit, cmdSnapshotList, cmdSnapshotParent)
(cmdSnapshotDelete, cmdDomainSnapshotRevert): Use it, adding an
option where needed.
* tools/virsh.pod (snapshot-delete, snapshot-edit)
(snapshot-list, snapshot-parent, snapshot-revert): Document
use of --current.
(snapshot-dumpxml): Mention alternative.
---

Does not strictly depend on virDomainSnapshotNumChildren, other
than the fact that I'm touching 'virsh snapshot-list dom --from name'
to add 'virsh snapshot-list dom --current'; I could split that
change out from the rest of the patch if desired to get the ease-of-use
in the other snapshot-* commands before the new API is approved.

 tools/virsh.c   |  109 ---
 tools/virsh.pod |   31 ++--
 2 files changed, 90 insertions(+), 50 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 86b230d..ea7b56b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12816,6 +12816,48 @@ cleanup:
 return ret;
 }

+/* Helper for resolving {--current | --ARG name} into a snapshot
+ * belonging to DOM.  If EXCLUSIVE, fail if both --current and arg are
+ * present.  On success, populate *SNAP, and if NAME is not NULL,
+ * *NAME, before returning 0.  On failure, return -1 after issuing an
+ * error message.  */
+static int
+vshLookupSnapshot(vshControl *ctl, const vshCmd *cmd,
+  const char *arg, bool exclusive, virDomainPtr dom,
+  virDomainSnapshotPtr *snap, const char **name)
+{
+bool current = vshCommandOptBool(cmd, current);
+const char *snapname = NULL;
+
+if (vshCommandOptString(cmd, arg, snapname)  0) {
+vshError(ctl, _(invalid argument for --%s), arg);
+return -1;
+}
+
+if (exclusive  current  snapname) {
+vshError(ctl, _(--%s and --current are mutually exclusive), arg);
+return -1;
+}
+
+if (snapname) {
+*snap = virDomainSnapshotLookupByName(dom, snapname, 0);
+} else if (current) {
+*snap = virDomainSnapshotCurrent(dom, 0);
+} else {
+vshError(ctl, _(--%s or --current is required), arg);
+return -1;
+}
+if (!*snap) {
+virshReportError(ctl);
+return -1;
+}
+
+if (name  *snap)
+*name = vshStrdup(ctl, virDomainSnapshotGetName(*snap));
+
+return 0;
+}
+
 /*
  * snapshot-edit command
  */
@@ -12827,7 +12869,7 @@ static const vshCmdInfo info_snapshot_edit[] = {

 static const vshCmdOptDef opts_snapshot_edit[] = {
 {domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
-{snapshotname, VSH_OT_DATA, VSH_OFLAG_REQ, N_(snapshot name)},
+{snapshotname, VSH_OT_DATA, 0, N_(snapshot name)},
 {current, VSH_OT_BOOL, 0, N_(also set edited snapshot as current)},
 {NULL, 0, 0, NULL}
 };
@@ -12845,21 +12887,19 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
 unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE;
 unsigned int define_flags = VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;

-if (vshCommandOptBool(cmd, current))
+if (vshCommandOptBool(cmd, current) 
+vshCommandOptBool(cmd, snapshotname))
 define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT;

 if (!vshConnectionUsability(ctl, ctl-conn))
 return false;

-if (vshCommandOptString(cmd, snapshotname, name) = 0)
-goto cleanup;
-
 dom = vshCommandOptDomain(ctl, cmd, NULL);
 if (dom == NULL)
 goto cleanup;

-snapshot = virDomainSnapshotLookupByName(dom, name, 0);
-if (snapshot == NULL)
+if (vshLookupSnapshot(ctl, cmd, snapshotname, false, dom,
+  snapshot, name)  0)
 goto cleanup;

 /* Get the XML configuration of the snapshot.  */
@@ -13109,6 +13149,8 @@ static const vshCmdOptDef opts_snapshot_list[] = {
  N_(list only snapshots that have metadata that would prevent undefine)},
 {tree, VSH_OT_BOOL, 0, N_(list snapshots in a tree)},
 {from, VSH_OT_DATA, 0, N_(limit list to children of given snapshot)},
+{current, VSH_OT_BOOL, 0,
+ N_(limit list to children of current snapshot)},
 {descendants, VSH_OT_BOOL, 0, N_(with --from, list all descendants)},
 {NULL, 0, 0, NULL}
 };
@@ -13142,10 +13184,17 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
 int start_index = -1;
 bool descendants = false;

-

Re: [libvirt] [PATCH] build: fix 'make distcheck' with pdwtags installed

2011-10-06 Thread Eric Blake 

On 10/06/2011 01:38 PM, Laine Stump wrote:

On 10/04/2011 12:33 PM, Eric Blake wrote:

I am getting this failure with 'make distcheck':

GEN ../../src/remote_protocol-structs
/bin/sh: ../../src/remote_protocol-structs-t: Permission denied
make[4]: *** [../../src/remote_protocol-structs] Error 1



ACK.


Thanks; pushed.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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