Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code

2012-02-24 Thread D. Herrendoerfer


On Feb 23, 2012, at 11:16 PM, Laine Stump wrote:


On 02/22/2012 08:17 AM, D. Herrendoerfer wrote:

From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name

Add de-association handling for 802.1qbg (vepa) via lldpad
netlink messages. Also adds the possibility to perform an
association request without waiting for a confirmation.


The main issue I see with this patch is with whitespace, but that can
easily be fixed prior to pushing it, so ACK. Is the message format  
used

by this patch, the absolute final version? (i.e. can we safely push it
an know that it will be correct?)


The patch to libvirt was picked this week. So yes, we can assume that  
the

message will not be changed. But as always : Never say never !

The callback mechanism is not re-armed when libvirt is restarted now.
The reason is: lldpad remembers who sent the associate by pid - since
in theory there could be multiple agents performing associations.
So if the libvirt pid changes, there is little we can do now.

And thanks !

Dirk H





Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
---
src/qemu/qemu_migration.c|2 +-
src/util/virnetdevmacvlan.c  |  315  
+-

src/util/virnetdevvportprofile.c |   33 +++--
src/util/virnetdevvportprofile.h |3 +-
4 files changed, 339 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 12cfbde..31428f8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2567,7 +2567,7 @@  
qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) {

   net-mac,

virDomainNetGetActualDirectDev(net),

   def-uuid,
-
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH)  0)
+
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false)  0)

goto err_exit;
}
last_good_net = i;
diff --git a/src/util/virnetdevmacvlan.c b/src/util/ 
virnetdevmacvlan.c

index 25d0846..b3e3325 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,  
VIR_NETDEV_MACVLAN_MODE_LAST,

  passthrough)

#if WITH_MACVTAP
-


spurious whitespace change.


# include stdint.h
# include stdio.h
# include errno.h
@@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,  
VIR_NETDEV_MACVLAN_MODE_LAST,

# include linux/if.h
# include linux/if_tun.h

+# include c-ctype.h
+
/* Older kernels lacked this enum value.  */
# if !HAVE_DECL_MACVLAN_MODE_PASSTHRU
#  define MACVLAN_MODE_PASSTHRU 8
@@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,  
VIR_NETDEV_MACVLAN_MODE_LAST,

# include virfile.h
# include virnetlink.h
# include virnetdev.h
+# include virpidfile.h
+

# define MACVTAP_NAME_PREFIXmacvtap
# define MACVTAP_NAME_PATTERN   macvtap%d
@@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,  
VIR_NETDEV_MACVLAN_MODE_LAST,

# define MACVLAN_NAME_PREFIXmacvlan
# define MACVLAN_NAME_PATTERN   macvlan%d

+


Another spurious whitespace change.


/**
 * virNetDevMacVLanCreate:
 *
@@ -445,6 +449,293 @@ static const uint32_t  
modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = {

[VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU,
};

+/* Struct to hold the state and configuration of a 802.1qbg port */
+struct virNetlinkCallbackData {
+   char cr_ifname[64];
+   virNetDevVPortProfilePtr virtPortProfile;
+   const unsigned char *macaddress;
+   const char *linkdev;
+   const unsigned char *vmuuid;
+   enum virNetDevVPortProfileOp vmOp;
+   unsigned int linkState;
+};
+
+typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr;
+
+#define INSTANCE_STRLEN 36
+
+static int instance2str(const unsigned char *p, char *dst, size_t  
size)

+{
+if (dst  size  INSTANCE_STRLEN) {
+snprintf(dst, size, %02x%02x%02x%02x-%02x%02x-%02x%02x-
+%02x%02x-%02x%02x%02x%02x%02x%02x,
+p[0], p[1], p[2], p[3],
+p[4], p[5], p[6], p[7],
+p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
+return 0;
+}
+return -1;
+}
+
+# define LLDPAD_PID_FILE  /var/run/lldpad.pid
+# define VIRIP_PID_FILE   /var/run/virip.pid
+
+/**
+ * virNetDevMacVLanVPortProfileCallback:
+ *
+ * @msg: The buffer containing the received netlink message
+ * @length: The length of the received netlink message.
+ * @peer: The netling sockaddr containing the peer information
+ * @handeled: Contains information if the message has been replied  
to yet
+ * @opaque: Contains vital information regarding the associated vm  
an interface

+ *
+ * This function is called when a netlink message is received. The  
function
+ * reads the message and responds if it is pertinent to the  
running VMs

+ * network interface.
+ */
+
+static void

Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code

2012-02-24 Thread D. Herrendoerfer


On Feb 24, 2012, at 3:29 PM, D. Herrendoerfer wrote:



On Feb 23, 2012, at 11:16 PM, Laine Stump wrote:


On 02/22/2012 08:17 AM, D. Herrendoerfer wrote:

From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name

Add de-association handling for 802.1qbg (vepa) via lldpad
netlink messages. Also adds the possibility to perform an
association request without waiting for a confirmation.


The main issue I see with this patch is with whitespace, but that can
easily be fixed prior to pushing it, so ACK. Is the message format  
used
by this patch, the absolute final version? (i.e. can we safely push  
it

an know that it will be correct?)


The patch to libvirt was picked this week. So yes, we can assume  
that the

message will not be changed. But as always : Never say never !


The patch to lldpad (Doh!) was picked ...



The callback mechanism is not re-armed when libvirt is restarted now.
The reason is: lldpad remembers who sent the associate by pid - since
in theory there could be multiple agents performing associations.
So if the libvirt pid changes, there is little we can do now.

And thanks !

Dirk H





Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
---
src/qemu/qemu_migration.c|2 +-
src/util/virnetdevmacvlan.c  |  315 +++ 
++-

src/util/virnetdevvportprofile.c |   33 +++--
src/util/virnetdevvportprofile.h |3 +-
4 files changed, 339 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 12cfbde..31428f8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2567,7 +2567,7 @@  
qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) {

  net-mac,
   
virDomainNetGetActualDirectDev(net),

  def-uuid,
-
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH)  0)
+
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false)  0)

   goto err_exit;
   }
   last_good_net = i;
diff --git a/src/util/virnetdevmacvlan.c b/src/util/ 
virnetdevmacvlan.c

index 25d0846..b3e3325 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,  
VIR_NETDEV_MACVLAN_MODE_LAST,

 passthrough)

#if WITH_MACVTAP
-


spurious whitespace change.


# include stdint.h
# include stdio.h
# include errno.h
@@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,  
VIR_NETDEV_MACVLAN_MODE_LAST,

# include linux/if.h
# include linux/if_tun.h

+# include c-ctype.h
+
/* Older kernels lacked this enum value.  */
# if !HAVE_DECL_MACVLAN_MODE_PASSTHRU
#  define MACVLAN_MODE_PASSTHRU 8
@@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,  
VIR_NETDEV_MACVLAN_MODE_LAST,

# include virfile.h
# include virnetlink.h
# include virnetdev.h
+# include virpidfile.h
+

# define MACVTAP_NAME_PREFIXmacvtap
# define MACVTAP_NAME_PATTERN   macvtap%d
@@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,  
VIR_NETDEV_MACVLAN_MODE_LAST,

# define MACVLAN_NAME_PREFIXmacvlan
# define MACVLAN_NAME_PATTERN   macvlan%d

+


Another spurious whitespace change.


/**
* virNetDevMacVLanCreate:
*
@@ -445,6 +449,293 @@ static const uint32_t  
modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = {

   [VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU,
};

+/* Struct to hold the state and configuration of a 802.1qbg port */
+struct virNetlinkCallbackData {
+   char cr_ifname[64];
+   virNetDevVPortProfilePtr virtPortProfile;
+   const unsigned char *macaddress;
+   const char *linkdev;
+   const unsigned char *vmuuid;
+   enum virNetDevVPortProfileOp vmOp;
+   unsigned int linkState;
+};
+
+typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr;
+
+#define INSTANCE_STRLEN 36
+
+static int instance2str(const unsigned char *p, char *dst, size_t  
size)

+{
+if (dst  size  INSTANCE_STRLEN) {
+snprintf(dst, size, %02x%02x%02x%02x-%02x%02x-%02x%02x-
+%02x%02x-%02x%02x%02x%02x%02x%02x,
+p[0], p[1], p[2], p[3],
+p[4], p[5], p[6], p[7],
+p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
+return 0;
+}
+return -1;
+}
+
+# define LLDPAD_PID_FILE  /var/run/lldpad.pid
+# define VIRIP_PID_FILE   /var/run/virip.pid
+
+/**
+ * virNetDevMacVLanVPortProfileCallback:
+ *
+ * @msg: The buffer containing the received netlink message
+ * @length: The length of the received netlink message.
+ * @peer: The netling sockaddr containing the peer information
+ * @handeled: Contains information if the message has been  
replied to yet
+ * @opaque: Contains vital information regarding the associated  
vm an interface

+ *
+ * This function is called when a netlink message is received.  
The function
+ * reads the message and 

[libvirt] [PATCH] Improve error reporting when virsh console is run without a TTY

2012-02-24 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

If attempting to run

  ssh root@somehost virsh console someguest

You'll get an error

  2012-02-15 13:11:47.683+: 4765: info : libvirt version: 0.9.10, package: 
1.fc18 (Unknown, 2012-02-15-11:48:57, lettuce.camlab.fab.redhat.com)
  2012-02-15 13:11:47.683+: 4765: error : vshRunConsole:320 : unable to get 
tty attributes: Invalid argument
  Connected to domain f16x86_64
  Escape character is ^]

There are several problems here

 - The actual error message is bad for users
 - We shouldn't rely on VIR_ERROR for this case
 - The prompt makes it look like we still connected
   because we didn't flush stdout.

* virsh.c: Flush stdout before starting console and check
  for a valid tty
---
 tools/virsh.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 3be86ed..b97a888 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -838,8 +838,14 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, const 
char *name)
 goto cleanup;
 }
 
+if (!isatty(STDIN_FILENO)) {
+vshError(ctl, %s, _(Cannot run interactive console without a 
controlling TTY));
+goto cleanup;
+}
+
 vshPrintExtra(ctl, _(Connected to domain %s\n), virDomainGetName(dom));
 vshPrintExtra(ctl, _(Escape character is %s\n), ctl-escapeChar);
+fflush(stdout);
 if (vshRunConsole(dom, name, ctl-escapeChar) == 0)
 ret = true;
 
-- 
1.7.7.6

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


[libvirt] [PATCH] Workaround python header file insanity

2012-02-24 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The /usr/include/python/pyconfig.h file pollutes the global
namespace with a huge number of HAVE_XXX and WITH_XXX
defines. These change what we detected in our own config.h
In particular if you try to build without DTrace, python's
headers turn it back on with predictable fail.

THe hack to workaround this is to rename WITH_DTRACE to
WITH_DTRACE_PROBES to avoid the namespace clash
---
 configure.ac   |4 ++--
 daemon/Makefile.am |2 +-
 src/Makefile.am|4 ++--
 src/internal.h |2 +-
 tests/Makefile.am  |2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index 732f4fe..262e63b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1378,10 +1378,10 @@ if test $with_dtrace != no ; then
 with_dtrace=yes
   fi
   if test $with_dtrace = yes; then
-AC_DEFINE_UNQUOTED([WITH_DTRACE], 1, [whether DTrace static probes are 
available])
+AC_DEFINE_UNQUOTED([WITH_DTRACE_PROBES], 1, [whether DTrace static probes 
are available])
   fi
 fi
-AM_CONDITIONAL([WITH_DTRACE], [test $with_dtrace != no])
+AM_CONDITIONAL([WITH_DTRACE_PROBES], [test $with_dtrace != no])
 
 
 dnl NUMA lib
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index e2c1357..db4abf5 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -109,7 +109,7 @@ libvirtd_LDADD =\
$(SASL_LIBS)\
$(POLKIT_LIBS)
 
-if WITH_DTRACE
+if WITH_DTRACE_PROBES
 libvirtd_LDADD += ../src/probes.o
 endif
 
diff --git a/src/Makefile.am b/src/Makefile.am
index d5f52a0..9b1921d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1284,7 +1284,7 @@ libvirt_la_CFLAGS = -DIN_LIBVIRT $(AM_CFLAGS)
 # picked out for us.
 libvirt_la_DEPENDENCIES = $(libvirt_la_BUILT_LIBADD) $(LIBVIRT_SYMBOL_FILE)
 
-if WITH_DTRACE
+if WITH_DTRACE_PROBES
 libvirt_la_BUILT_LIBADD += probes.o
 libvirt_la_DEPENDENCIES += probes.o
 nodist_libvirt_la_SOURCES = probes.h
@@ -1521,7 +1521,7 @@ libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \
$(LIBNL_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
$(RT_LIBS) \
../gnulib/lib/libgnu.la
-if WITH_DTRACE
+if WITH_DTRACE_PROBES
 libvirt_lxc_LDADD += probes.o
 endif
 if WITH_SECDRIVER_SELINUX
diff --git a/src/internal.h b/src/internal.h
index fabcb52..3408541 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -247,7 +247,7 @@
 # define VIR_DIV_UP(value, size) (((value) + (size) - 1) / (size))
 
 
-# if WITH_DTRACE
+# if WITH_DTRACE_PROBES
 #  ifndef LIBVIRT_PROBES_H
 #   define LIBVIRT_PROBES_H
 #   include probes.h
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3fb9e2f..9974c2f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -29,7 +29,7 @@ INCLUDES += \
 endif
 
 PROBES_O =
-if WITH_DTRACE
+if WITH_DTRACE_PROBES
 PROBES_O += ../src/probes.o
 endif
 
-- 
1.7.7.6

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


Re: [libvirt] [PATCH] Workaround python header file insanity

2012-02-24 Thread Eric Blake
On 02/24/2012 08:13 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The /usr/include/python/pyconfig.h file pollutes the global
 namespace with a huge number of HAVE_XXX and WITH_XXX
 defines. These change what we detected in our own config.h
 In particular if you try to build without DTrace, python's
 headers turn it back on with predictable fail.
 
 THe hack to workaround this is to rename WITH_DTRACE to
 WITH_DTRACE_PROBES to avoid the namespace clash
 ---
  configure.ac   |4 ++--
  daemon/Makefile.am |2 +-
  src/Makefile.am|4 ++--
  src/internal.h |2 +-
  tests/Makefile.am  |2 +-
  5 files changed, 7 insertions(+), 7 deletions(-)

ACK.  (And I really wish python would clean up their act).

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



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

Re: [libvirt] [PATCH] Improve error reporting when virsh console is run without a TTY

2012-02-24 Thread Eric Blake
On 02/24/2012 08:13 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 If attempting to run
 
   ssh root@somehost virsh console someguest
 
 You'll get an error
 
   2012-02-15 13:11:47.683+: 4765: info : libvirt version: 0.9.10, 
 package: 1.fc18 (Unknown, 2012-02-15-11:48:57, lettuce.camlab.fab.redhat.com)
   2012-02-15 13:11:47.683+: 4765: error : vshRunConsole:320 : unable to 
 get tty attributes: Invalid argument
   Connected to domain f16x86_64
   Escape character is ^]
 
 There are several problems here
 
  - The actual error message is bad for users
  - We shouldn't rely on VIR_ERROR for this case
  - The prompt makes it look like we still connected
because we didn't flush stdout.

Did you also test 'ssh root@somehost virsh start --console someguest' to
make sure that the guest is started, but the console sanely refused?

 +++ b/tools/virsh.c
 @@ -838,8 +838,14 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, const 
 char *name)
  goto cleanup;
  }
  
 +if (!isatty(STDIN_FILENO)) {
 +vshError(ctl, %s, _(Cannot run interactive console without a 
 controlling TTY));
 +goto cleanup;
 +}
 +
  vshPrintExtra(ctl, _(Connected to domain %s\n), virDomainGetName(dom));
  vshPrintExtra(ctl, _(Escape character is %s\n), ctl-escapeChar);
 +fflush(stdout);

At any rate, this looks sane. ACK.

Hmm, we ought to ask gnulib if they will relicense the isatty() module
(currently LGPLv3+); as isatty() on mingw has bugs worth working around.

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



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

Re: [libvirt] [PATCHv2] Error out when using SPICE TLS with spice_tls=0

2012-02-24 Thread Eric Blake
On 02/24/2012 03:41 AM, Daniel P. Berrange wrote:
 On Fri, Feb 24, 2012 at 11:34:45AM +0100, Christophe Fergeau wrote:
 It's possible to disable SPICE TLS in qemu.conf. When this happens,
 libvirt ignores any SPICE TLS port or x509 directory that may have
 been set when it builds the qemu command line to use. However, it's
 not ignoring the secure channels that may have been set and adds
 tls-channel arguments to qemu command line.
 Current qemu versions don't report an error when this happens, and try to use
 TLS for the specified channels.



 This fixes bug #790436

Thanks for chasing this down.

 ACK, if we  s/XML_ERROR/CONFIG_UNSUPPORTED/  in the those two error
 messages

Changed and pushed.

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



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

[libvirt] New QMP event interface (was Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event)

2012-02-24 Thread Anthony Liguori

On 02/24/2012 10:56 AM, Luiz Capitulino wrote:

On Fri, 24 Feb 2012 10:44:11 -0600
Anthony Liguorianth...@codemonkey.ws  wrote:


I'm asking because the conversion of events to the qapi is not too far away,
but I think that using QOM will somewhat deprecate the code you have in the
glib branch (besides having to wait for 1.2)?


I have some vague ideas about what to do here.  One thought would be to have a
standard notifier mechanism in Object that was advertised as a property type.
We could then provide an interface via QMP to [un]subscribe to a notifier 
property.


This seems to be a good match with your previous ideas, implemented in the glib
branch. But then subsystems/devices emitting events will have to be converted
to QOM first...


Well we need to keep the old events for compatibility, so it's really just about 
new events.  I think that by end of 1.2, we would have all non-qdev subsystems 
converted to QOM also so this shouldn't be a problem in practice.





I won't get to this until the 1.2 time frame though.  My goals for 1.1 are to
get qbus conversions merged and refactor IRQs/MemoryRegions to use QOM.  If time
permits, also refactor the PC to better use QOM.

If someone wants to tackle events in QOM, I'd be happy to provide some
suggestions on where to start.


I'd like to hear about it, but I'm not sure when I'll start working on it.


I've only thought about this roughly, but the problems that need to be solved 
are:

1) Have an object property that corresponds to a NotifierList (easy)

2) Figure out what it means to get and set a NotifierList.  I think perhaps 
we could somehow turn this into subscribe/unsubscribe...


We could take a plan9-like approach where get would return the canonical path 
of a new object that corresponded to a subscription to the event.  This is nice 
because unsubscribing then just becomes a matter of destroying the subscription 
object.


Once you had this subscription object, I think we would want a mechanism to 
connect the subscription with either a native function pointer or some 
mechanism that would let us translate that to QMP.  Maybe we only connect with a 
native function pointer and use QAPI generation code to build a native function 
pointer that spits out a marshalled QObject.


3) We would need to think through the QMP interface for all of this.  Given a 
path, we want to be able to subscribe to an event and unsubscribe from an event. 
 We need to unsubscribe all subscribed events when the connection is lost.  It 
would be nice to have convenience interfaces for doing things like subscribing 
to any event on a given type too including yet to be created objects.


Cc'ing libvirt here to see if they have any additional requirements.

Regards,

Anthony Liguori

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


[libvirt] [PATCH v3] Fixed URI parsing

2012-02-24 Thread Martin Kletzander
Function xmlParseURI does not remove square brackets around IPv6
address when parsing. One of the solutions is making wrappers around
functions working with xmlURI*. This assures that uri-server will be
always properly assigned and it doesn't have to be changed when used
on some new place in the code.
For this purpose, functions virParseURI and virSaveURI were
added. These function are wrappers around xmlParseURI and xmlSaveUri
respectively.
Also there is one new syntax check function to prohibit these functions
anywhere else.

File changes:
 - src/util/viruri.h-- declaration
 - src/util/viruri.c-- definition
 - src/libvirt_private.syms -- symbol export
 - src/Makefile.am  -- added source and header files
 - cfg.mk   -- added sc_prohibit_xmlURI
 - all others   -- ID name and include fixes
---
v3:
 - wrappers moved to new files
 - syntax check added
 - virAsprintf used instead of nasty memory mechanics

v2:
 - added virSaveURI for building back the original string

 cfg.mk |8 
 src/Makefile.am|3 +-
 src/datatypes.h|2 +-
 src/driver.h   |2 +-
 src/esx/esx_driver.c   |5 +-
 src/esx/esx_util.c |2 +-
 src/esx/esx_util.h |4 +-
 src/hyperv/hyperv_util.c   |2 +-
 src/hyperv/hyperv_util.h   |5 +-
 src/libvirt.c  |   10 ++--
 src/libvirt_private.syms   |5 ++
 src/libxl/libxl_driver.c   |3 +-
 src/lxc/lxc_driver.c   |3 +-
 src/openvz/openvz_driver.c |3 +-
 src/qemu/qemu_driver.c |2 +-
 src/qemu/qemu_migration.c  |7 ++-
 src/remote/remote_driver.c |9 ++--
 src/uml/uml_driver.c   |3 +-
 src/util/qparams.c |3 +-
 src/util/viruri.c  |   91 
 src/util/viruri.h  |   18 +
 src/vbox/vbox_tmpl.c   |3 +-
 src/vmx/vmx.c  |6 +-
 src/xen/xen_driver.c   |4 +-
 src/xen/xen_hypervisor.h   |3 +-
 src/xen/xend_internal.c|4 +-
 src/xen/xend_internal.h|2 +-
 src/xenapi/xenapi_driver.c |2 +-
 src/xenapi/xenapi_utils.c  |4 +-
 src/xenapi/xenapi_utils.h  |4 +-
 30 files changed, 174 insertions(+), 48 deletions(-)
 create mode 100644 src/util/viruri.c
 create mode 100644 src/util/viruri.h

diff --git a/cfg.mk b/cfg.mk
index dcf44bb..9759d87 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -457,6 +457,12 @@ sc_prohibit_xmlGetProp:
halt='use virXMLPropString, not xmlGetProp' \
  $(_sc_search_regexp)

+# xml(ParseURI|SaveUri) doesn't handle IPv6 URIs well
+sc_prohibit_xmlURI:
+   @prohibit='\xml(ParseURI|SaveUri) *\(' \
+   halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)'  \
+ $(_sc_search_regexp)
+
 # ATTRIBUTE_UNUSED should only be applied in implementations, not
 # header declarations
 sc_avoid_attribute_unused_in_header:
@@ -758,6 +764,8 @@ exclude_file_name_regexp--sc_prohibit_strncpy = \

 exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/xml\.c$$

+exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$
+
 exclude_file_name_regexp--sc_require_config_h = ^examples/

 exclude_file_name_regexp--sc_require_config_h_first = ^examples/
diff --git a/src/Makefile.am b/src/Makefile.am
index d5f52a0..e2542b1 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -104,7 +104,8 @@ UTIL_SOURCES =  
\
util/virnetlink.c util/virnetlink.h \
util/virrandom.h util/virrandom.c   \
util/virsocketaddr.h util/virsocketaddr.c \
-   util/virtime.h util/virtime.c
+   util/virtime.h util/virtime.c \
+   util/viruri.h util/viruri.c

 EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
$(srcdir)/util/virkeycode-mapgen.py
diff --git a/src/datatypes.h b/src/datatypes.h
index 47058ed..fc284d2 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -151,7 +151,7 @@ struct _virConnect {
  */
 unsigned int magic; /* specific value to check */
 unsigned int flags; /* a set of connection flags */
-xmlURIPtr uri;  /* connection URI */
+virURIPtr uri;  /* connection URI */

 /* The underlying hypervisor driver and network driver. */
 virDriverPtr  driver;
diff --git a/src/driver.h b/src/driver.h
index d27fa99..b04b254 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -9,9 +9,9 @@
 # include config.h

 # include unistd.h
-# include libxml/uri.h

 # include internal.h
+# include viruri.h
 /*
  * List of registered drivers numbers
  */
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f5e1cc7..b6b22f8 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -44,6 +44,7 @@
 #include esx_vi.h
 #include esx_vi_methods.h
 #include esx_util.h
+#include 

Re: [libvirt] [PATCH v3] Fixed URI parsing

2012-02-24 Thread Eric Blake
On 02/24/2012 11:09 AM, Martin Kletzander wrote:
 Function xmlParseURI does not remove square brackets around IPv6
 address when parsing. One of the solutions is making wrappers around
 functions working with xmlURI*. This assures that uri-server will be
 always properly assigned and it doesn't have to be changed when used
 on some new place in the code.
 For this purpose, functions virParseURI and virSaveURI were
 added. These function are wrappers around xmlParseURI and xmlSaveUri
 respectively.
 Also there is one new syntax check function to prohibit these functions
 anywhere else.
 

 +++ b/src/libvirt_private.syms
 @@ -1430,6 +1430,11 @@ virTypedParameterArrayValidate;
  virTypedParameterAssign;
 
 
 +# viruri.h
 +virURIParse;
 +virURIFormat;

Swap these two lines.

 +xmlURIPtr
 +virURIParse(const char *uri)
 +{
 +xmlURIPtr ret = xmlParseURI(uri);
 +
 +/* First check: does it even make sense to jump inside */
 +if (ret != NULL 
 +ret-server != NULL 
 +ret-server[0] == '[') {
 +size_t length = strlen(ret-server);
 +
 +/* We want to modify the server string only if there are
 + * square brackets on both ends and inside there is IPv6
 + * address. Otherwise we could make a mistake by modifying
 + * something else than IPv6 address. */

s/else than/other than an/

 +unsigned char *
 +virURIFormat(xmlURIPtr uri)
 +{
 +char *tmpserver = NULL, *backupserver = uri-server;

NULL deref...

 +unsigned char *ret;
 +
 +/* First check: does it make sense to do anything */
 +if (uri != NULL 

...since you allow uri == NULL on input.  Reorder the assignment to
backupserver to come after you know uri is not NULL.

 +uri-server != NULL 
 +strchr(uri-server, ':') != NULL) {
 +
 +if (virAsprintf(tmpserver, [%s], uri-server) == -1)

It's more idiomatic to use ' 0', not '== -1'.

 +++ b/src/util/viruri.h
 @@ -0,0 +1,18 @@
 +/*
 + * viruri.h: internal definitions used for URI parsing.

Needs a copyright header.

ACK with those nits fixed; I think we're close enough that you can push
without having to get a review on a v4.

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



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

Re: [libvirt] Virt io blk buffer

2012-02-24 Thread Eric Blake
On 02/23/2012 11:53 PM, Pankaj Rawat wrote:
 Hi all 
 
 Can any one tell what is the default Vitio blk IO Buffer size 

You'll probably get better response on the qemu lists, since virtio is
part of qemu.

 
 And can we change it ? if yes how?

Libvirt does not currently expose anything to change this, other than
the fact that _if_ qemu supports a way to change it on the command line,
you can use qemu:command in your XML to add the extra arguments; or if
qemu supports it via a monitor command, you can use 'virsh
qemu-monitor-command' to change it at runtime.  But again, I don't know
enough about the situation to know if qemu even exposes a
parameterizable block size for virtio, since no one has previously asked
for libvirt to start encoding it into its XML as a formal feature.

 
 The contents of this e-mail and any attachment(s) are confidential and

Disclaimers like this are unenforceable when posting to publicly
archived lists, and it is considered poor netiquette to leave them in.
I'd recommend sending from a private personal account if your employer
insists on adding this garbage to your mail from work.

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



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

[libvirt] [PATCH] Fixed URI parsing

2012-02-24 Thread Martin Kletzander
Function xmlParseURI does not remove square brackets around IPv6
address when parsing. One of the solutions is making wrappers around
functions working with xmlURI*. This assures that uri-server will be
always properly assigned and it doesn't have to be changed when used
on some new place in the code.
For this purpose, functions virParseURI and virSaveURI were
added. These function are wrappers around xmlParseURI and xmlSaveUri
respectively.
Also there is one new syntax check function to prohibit these functions
anywhere else.

File changes:
 - src/util/viruri.h-- declaration
 - src/util/viruri.c-- definition
 - src/libvirt_private.syms -- symbol export
 - src/Makefile.am  -- added source and header files
 - cfg.mk   -- added sc_prohibit_xmlURI
 - all others   -- ID name and include fixes
---
v4:
 - fixed NULL pointer dereference
 - fixed minor typo
 - copyright added to header

v3:
 - wrappers moved to new files
 - syntax check added
 - virAsprintf used instead of nasty memory mechanics

v2:
 - added virSaveURI for building back the original string

 cfg.mk |8 
 src/Makefile.am|3 +-
 src/datatypes.h|2 +-
 src/driver.h   |2 +-
 src/esx/esx_driver.c   |5 +-
 src/esx/esx_util.c |2 +-
 src/esx/esx_util.h |4 +-
 src/hyperv/hyperv_util.c   |2 +-
 src/hyperv/hyperv_util.h   |5 +-
 src/libvirt.c  |   10 ++--
 src/libvirt_private.syms   |5 ++
 src/libxl/libxl_driver.c   |3 +-
 src/lxc/lxc_driver.c   |3 +-
 src/openvz/openvz_driver.c |3 +-
 src/qemu/qemu_driver.c |2 +-
 src/qemu/qemu_migration.c  |7 ++-
 src/remote/remote_driver.c |9 ++--
 src/uml/uml_driver.c   |3 +-
 src/util/qparams.c |3 +-
 src/util/viruri.c  |   93 
 src/util/viruri.h  |   22 ++
 src/vbox/vbox_tmpl.c   |3 +-
 src/vmx/vmx.c  |6 +-
 src/xen/xen_driver.c   |4 +-
 src/xen/xen_hypervisor.h   |3 +-
 src/xen/xend_internal.c|4 +-
 src/xen/xend_internal.h|2 +-
 src/xenapi/xenapi_driver.c |2 +-
 src/xenapi/xenapi_utils.c  |4 +-
 src/xenapi/xenapi_utils.h  |4 +-
 30 files changed, 180 insertions(+), 48 deletions(-)
 create mode 100644 src/util/viruri.c
 create mode 100644 src/util/viruri.h

diff --git a/cfg.mk b/cfg.mk
index dcf44bb..9759d87 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -457,6 +457,12 @@ sc_prohibit_xmlGetProp:
halt='use virXMLPropString, not xmlGetProp' \
  $(_sc_search_regexp)

+# xml(ParseURI|SaveUri) doesn't handle IPv6 URIs well
+sc_prohibit_xmlURI:
+   @prohibit='\xml(ParseURI|SaveUri) *\(' \
+   halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)'  \
+ $(_sc_search_regexp)
+
 # ATTRIBUTE_UNUSED should only be applied in implementations, not
 # header declarations
 sc_avoid_attribute_unused_in_header:
@@ -758,6 +764,8 @@ exclude_file_name_regexp--sc_prohibit_strncpy = \

 exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/xml\.c$$

+exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$
+
 exclude_file_name_regexp--sc_require_config_h = ^examples/

 exclude_file_name_regexp--sc_require_config_h_first = ^examples/
diff --git a/src/Makefile.am b/src/Makefile.am
index d5f52a0..e2542b1 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -104,7 +104,8 @@ UTIL_SOURCES =  
\
util/virnetlink.c util/virnetlink.h \
util/virrandom.h util/virrandom.c   \
util/virsocketaddr.h util/virsocketaddr.c \
-   util/virtime.h util/virtime.c
+   util/virtime.h util/virtime.c \
+   util/viruri.h util/viruri.c

 EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
$(srcdir)/util/virkeycode-mapgen.py
diff --git a/src/datatypes.h b/src/datatypes.h
index 47058ed..fc284d2 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -151,7 +151,7 @@ struct _virConnect {
  */
 unsigned int magic; /* specific value to check */
 unsigned int flags; /* a set of connection flags */
-xmlURIPtr uri;  /* connection URI */
+virURIPtr uri;  /* connection URI */

 /* The underlying hypervisor driver and network driver. */
 virDriverPtr  driver;
diff --git a/src/driver.h b/src/driver.h
index d27fa99..b04b254 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -9,9 +9,9 @@
 # include config.h

 # include unistd.h
-# include libxml/uri.h

 # include internal.h
+# include viruri.h
 /*
  * List of registered drivers numbers
  */
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f5e1cc7..b6b22f8 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -44,6 

Re: [libvirt] Correct a check for capacity arg of storageVolumeResize()

2012-02-24 Thread Eric Blake
On 02/23/2012 08:00 PM, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org
 
 ---
  src/storage/storage_driver.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
 index df0e291..641944d 100644
 --- a/src/storage/storage_driver.c
 +++ b/src/storage/storage_driver.c
 @@ -1758,7 +1758,7 @@ storageVolumeResize(virStorageVolPtr obj,
  goto out;
  }
  
 -if (abs_capacity  vol-allocation + pool-def-available) {
 +if (abs_capacity  vol-capacity + pool-def-available) {

I'm not sure this is right.  If you allow for sparse files and
over-commitment of capacity, then sparsely resizing to something that
has too much capacity but still fits within allocation limits would be a
reasonable that should not be rejected.  I think it is more likely that
we have several bugs in this area, in being too lax about which values
we are actually checking.  In fact, is the only reason we are checking
for overflow is to avoid wasting time doing a partial allocation just to
learn at the end that we would hit ENOSPACE?

Adding to your commit message with the actual scenario you used to
trigger the problem, and why your fix is correct, will go a long way in
convincing me that this is needed.

  virStorageReportError(VIR_ERR_INVALID_ARG,

Meanwhile, I just noticed this error code is wrong - the argument was
syntactically valid, so the real problem is that we are out of space,
which fits better as VIR_ERR_OPERATION_FAILED, or possibly a new error
message specific to out-of-space errors.

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



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

Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code

2012-02-24 Thread Laine Stump
While going through this code to clean up the white-space problems, I
found 3 issues that need to be addressed before I can push it. Sorry I
missed these before.

If you can base the new (and I hope final! :-) version on the version
where I've already corrected the whitespace, that would be very helpful
(otherwise I'll need to merge back with my updated version, a
mechanical, but time consuming process). Rather than post them to the
mailing list, I've pushed my updated versions of your patches to the
lldpad2 branch of my staging repo on gitorious:

  git://gitorious.org/~laine/libvirt/laine-staging.git

You should be able to get that branch into your local repo like this:

  git remote add laine-staging \
  git://gitorious.org/~laine/libvirt/laine-staging.git
  git fetch laine-staging
  git checkout laine-staging/llpad2

You can then cherry-pick those commits into your own branch, or create a
local branch based on it, then just squash in changes addressing the three 
problems I point out below.

Once these are fixed, I will ACK and push it.

Thanks!



On 02/22/2012 08:17 AM, D. Herrendoerfer wrote:
 From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name

 Add de-association handling for 802.1qbg (vepa) via lldpad
 netlink messages. Also adds the possibility to perform an
 association request without waiting for a confirmation.

 Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
 ---
  src/qemu/qemu_migration.c|2 +-
  src/util/virnetdevmacvlan.c  |  315 
 +-
  src/util/virnetdevvportprofile.c |   33 +++--
  src/util/virnetdevvportprofile.h |3 +-
  4 files changed, 339 insertions(+), 14 deletions(-)

 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index 12cfbde..31428f8 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -2567,7 +2567,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr 
 def) {
 net-mac,
 
 virDomainNetGetActualDirectDev(net),
 def-uuid,
 -   
 VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH)  0)
 +   
 VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false)  0)
  goto err_exit;
  }
  last_good_net = i;
 diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
 index 25d0846..b3e3325 100644
 --- a/src/util/virnetdevmacvlan.c
 +++ b/src/util/virnetdevmacvlan.c
 @@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, 
 VIR_NETDEV_MACVLAN_MODE_LAST,
passthrough)
  
  #if WITH_MACVTAP
 -
  # include stdint.h
  # include stdio.h
  # include errno.h
 @@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, 
 VIR_NETDEV_MACVLAN_MODE_LAST,
  # include linux/if.h
  # include linux/if_tun.h
  
 +# include c-ctype.h
 +
  /* Older kernels lacked this enum value.  */
  # if !HAVE_DECL_MACVLAN_MODE_PASSTHRU
  #  define MACVLAN_MODE_PASSTHRU 8
 @@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, 
 VIR_NETDEV_MACVLAN_MODE_LAST,
  # include virfile.h
  # include virnetlink.h
  # include virnetdev.h
 +# include virpidfile.h
 +
  
  # define MACVTAP_NAME_PREFIX macvtap
  # define MACVTAP_NAME_PATTERNmacvtap%d
 @@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, 
 VIR_NETDEV_MACVLAN_MODE_LAST,
  # define MACVLAN_NAME_PREFIX macvlan
  # define MACVLAN_NAME_PATTERNmacvlan%d
  
 +
  /**
   * virNetDevMacVLanCreate:
   *
 @@ -445,6 +449,293 @@ static const uint32_t 
 modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = {
  [VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU,
  };
  
 +/* Struct to hold the state and configuration of a 802.1qbg port */
 +struct virNetlinkCallbackData {
 + char cr_ifname[64];

Issue 1, part 1:

This should either be IFNAMSIZ bytes long, or be a char* (pointing to
separately allocated data) instead of a fixed size. This string is
copied into strings that are sent to the kernel for interface names, and
interface names have a maximum size of IFNAMSIZE (16); if you need
consistency of the name in all places (even in log messages), then you
should limit it at the source, but otherwise there's no sense in
checking string sizes both at this level and then again at the lower level.

My preference would be a char*, I think - leave it to the lower levels
to decide if it needs to be chopped.


 + virNetDevVPortProfilePtr virtPortProfile;
 + const unsigned char *macaddress;
 + const char *linkdev;
 + const unsigned char *vmuuid;
 + enum virNetDevVPortProfileOp vmOp;
 + unsigned int linkState;
 +};
 +
 +typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr;
 +
 +#define INSTANCE_STRLEN 36
 +
 +static int instance2str(const unsigned char *p, char *dst, size_t size)
 +{
 +if (dst  size  INSTANCE_STRLEN) {
 +

[libvirt] [PATCH] virsh: fix informational message in iface-bridge command

2012-02-24 Thread Laine Stump
See: https://bugzilla.redhat.com/show_bug.cgi?id=797066

The position of the bridge name and ethernet device name were
accidentally swapped in the message informing of success creating the
bridge.
---
 tools/virsh.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index b97a888..66bbb0c 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -8974,7 +8974,7 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd)
 }
 
 vshPrint(ctl, _(Created bridge %s with attached device %s\n),
- if_name, br_name);
+ br_name, if_name);
 
 /* start it up unless requested not to */
 if (!nostart) {
-- 
1.7.7.6

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


Re: [libvirt] [PATCH] virsh: fix informational message in iface-bridge command

2012-02-24 Thread Laine Stump
On 02/24/2012 02:34 PM, Laine Stump wrote:
 See: https://bugzilla.redhat.com/show_bug.cgi?id=797066

 The position of the bridge name and ethernet device name were
 accidentally swapped in the message informing of success creating the
 bridge.
 ---
  tools/virsh.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/tools/virsh.c b/tools/virsh.c
 index b97a888..66bbb0c 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -8974,7 +8974,7 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd)
  }
  
  vshPrint(ctl, _(Created bridge %s with attached device %s\n),
 - if_name, br_name);
 + br_name, if_name);
  
  /* start it up unless requested not to */
  if (!nostart) {

I pushed this as trivial :-)

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


Re: [libvirt] [PATCH v3] Fixed URI parsing

2012-02-24 Thread Daniel P. Berrange
On Fri, Feb 24, 2012 at 07:09:49PM +0100, Martin Kletzander wrote:
 +xmlURIPtr
 +virURIParse(const char *uri)

 +unsigned char *
 +virURIFormat(xmlURIPtr uri)

The data types here are wrong compared to the header.

Also the return value should not be unsigned - that is
libxml2 bad practice we shouldn't copy

 diff --git a/src/util/viruri.h b/src/util/viruri.h
 new file mode 100644
 index 000..1315488
 --- /dev/null
 +++ b/src/util/viruri.h
 @@ -0,0 +1,18 @@
 +/*
 + * viruri.h: internal definitions used for URI parsing.
 + */
 +
 +#ifndef __VIR_URI_H__
 +# define __VIR_URI_H__
 +
 +# include libxml/uri.h
 +
 +# include internal.h
 +
 +typedef xmlURIvirURI;
 +typedef xmlURIPtr virURIPtr;
 +
 +virURIPtrvirURIParse(const char *uri);
 +unsigned char *  virURIFormat(virURIPtr uri);
 +
 +#endif /* __VIR_URI_H__ */


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] Correct a check for capacity arg of storageVolumeResize()

2012-02-24 Thread Zeeshan Ali (Khattak)
On Fri, Feb 24, 2012 at 8:58 PM, Eric Blake ebl...@redhat.com wrote:
 On 02/23/2012 08:00 PM, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org

 ---
  src/storage/storage_driver.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
 index df0e291..641944d 100644
 --- a/src/storage/storage_driver.c
 +++ b/src/storage/storage_driver.c
 @@ -1758,7 +1758,7 @@ storageVolumeResize(virStorageVolPtr obj,
          goto out;
      }

 -    if (abs_capacity  vol-allocation + pool-def-available) {
 +    if (abs_capacity  vol-capacity + pool-def-available) {

 I'm not sure this is right.  If you allow for sparse files and
 over-commitment of capacity, then sparsely resizing to something that
 has too much capacity but still fits within allocation limits would be a
 reasonable that should not be rejected.  I think it is more likely that
 we have several bugs in this area, in being too lax about which values
 we are actually checking.  In fact, is the only reason we are checking
 for overflow is to avoid wasting time doing a partial allocation just to
 learn at the end that we would hit ENOSPACE?

 Adding to your commit message with the actual scenario you used to
 trigger the problem, and why your fix is correct, will go a long way in
 convincing me that this is needed.

I got a volume with '1G' allocation and '10G' capacity. The available
space in the parent pool is '5G'. With the current check for
overcapacity, I can only try to resize to = '6G'. You see the
problem?

          virStorageReportError(VIR_ERR_INVALID_ARG,

 Meanwhile, I just noticed this error code is wrong - the argument was
 syntactically valid, so the real problem is that we are out of space,
 which fits better as VIR_ERR_OPERATION_FAILED, or possibly a new error
 message specific to out-of-space errors.

K, i'll submit a fix for that.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

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

Re: [libvirt] [PATCH] Fixed service handling in specfile

2012-02-24 Thread Eric Blake
On 02/24/2012 04:28 AM, Martin Kletzander wrote:
 After adding the libvirt-guests service into usual runlevels, we used
 to start the libvirt-guests service. However this is usually not a
 good practice. As mentioned on fedoraproject wiki, the installations
 can be in changeroots, in an installer context, or in other situations
 where we don't want the services autostarted.
 ---
  libvirt.spec.in |8 
  1 files changed, 0 insertions(+), 8 deletions(-)
 
 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index 67cde23..072fd8e 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -1027,14 +1027,6 @@ fi
  %if %{with_systemd}
  %else
  /sbin/chkconfig --add libvirt-guests
 -if [ $1 -ge 1 ]; then
 -level=$(/sbin/runlevel | /bin/cut -d ' ' -f 2)
 -if /sbin/chkconfig --levels $level libvirt-guests; then
 -# this doesn't do anything but allowing for libvirt-guests to be
 -# stopped on the first shutdown
 -/sbin/service libvirt-guests start  /dev/null 21 || true
 -fi
 -fi
  %endif

ACK and pushed.

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



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

Re: [libvirt] [PATCH] Fixed URI parsing

2012-02-24 Thread Eric Blake
On 02/24/2012 11:48 AM, Martin Kletzander wrote:
 Function xmlParseURI does not remove square brackets around IPv6
 address when parsing. One of the solutions is making wrappers around
 functions working with xmlURI*. This assures that uri-server will be
 always properly assigned and it doesn't have to be changed when used
 on some new place in the code.
 For this purpose, functions virParseURI and virSaveURI were
 added. These function are wrappers around xmlParseURI and xmlSaveUri
 respectively.
 Also there is one new syntax check function to prohibit these functions
 anywhere else.
 
 File changes:
  - src/util/viruri.h-- declaration
  - src/util/viruri.c-- definition
  - src/libvirt_private.syms -- symbol export
  - src/Makefile.am  -- added source and header files
  - cfg.mk   -- added sc_prohibit_xmlURI
  - all others   -- ID name and include fixes

 +++ b/cfg.mk
 @@ -457,6 +457,12 @@ sc_prohibit_xmlGetProp:
   halt='use virXMLPropString, not xmlGetProp' \
 $(_sc_search_regexp)
 
 +# xml(ParseURI|SaveUri) doesn't handle IPv6 URIs well
 +sc_prohibit_xmlURI:
 + @prohibit='\xml(ParseURI|SaveUri) *\(' \
 + halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)'  \
 +   $(_sc_search_regexp)
 +
  # ATTRIBUTE_UNUSED should only be applied in implementations, not
  # header declarations
  sc_avoid_attribute_unused_in_header:
 @@ -758,6 +764,8 @@ exclude_file_name_regexp--sc_prohibit_strncpy = \
 
  exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/xml\.c$$
 
 +exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$
 +

ACK; and pushed with these further changes squashed in to address Dan's
comments on v3.

diff --git i/src/util/viruri.c w/src/util/viruri.c
index 0cbfc5a..6c4dfe3 100644
--- i/src/util/viruri.c
+++ w/src/util/viruri.c
@@ -25,10 +25,10 @@
  *
  * @returns the parsed uri object with some fixes
  */
-xmlURIPtr
+virURIPtr
 virURIParse(const char *uri)
 {
-xmlURIPtr ret = xmlParseURI(uri);
+virURIPtr ret = xmlParseURI(uri);

 /* First check: does it even make sense to jump inside */
 if (ret != NULL 
@@ -62,12 +62,12 @@ virURIParse(const char *uri)
  *
  * @returns the constructed uri as a string
  */
-unsigned char *
+char *
 virURIFormat(xmlURIPtr uri)
 {
 char *backupserver = NULL;
 char *tmpserver = NULL;
-unsigned char *ret;
+char *ret;

 /* First check: does it make sense to do anything */
 if (uri != NULL 
@@ -81,7 +81,7 @@ virURIFormat(xmlURIPtr uri)
 uri-server = tmpserver;
 }

-ret = xmlSaveUri(uri);
+ret = (char *) xmlSaveUri(uri);

 /* Put the fixed version back */
 if (tmpserver) {
diff --git i/src/util/viruri.h w/src/util/viruri.h
index b2b0ca8..5215e42 100644
--- i/src/util/viruri.h
+++ w/src/util/viruri.h
@@ -16,7 +16,7 @@
 typedef xmlURIvirURI;
 typedef xmlURIPtr virURIPtr;

-virURIPtrvirURIParse(const char *uri);
-unsigned char *  virURIFormat(virURIPtr uri);
+virURIPtr virURIParse(const char *uri);
+char *virURIFormat(virURIPtr uri);

 #endif /* __VIR_URI_H__ */
diff --git i/src/libvirt.c w/src/libvirt.c
index a4ee63d..cbb4119 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -1729,7 +1729,7 @@ virConnectGetURI (virConnectPtr conn)
 return NULL;
 }

-name = (char *)virURIFormat(conn-uri);
+name = virURIFormat(conn-uri);
 if (!name) {
 virReportOOMError();
 goto error;
diff --git i/src/remote/remote_driver.c w/src/remote/remote_driver.c
index 8fb46e1..bcd78ee 100644
--- i/src/remote/remote_driver.c
+++ w/src/remote/remote_driver.c
@@ -504,7 +504,7 @@ doRemoteOpen (virConnectPtr conn,
 transport_str[-1] = '\0';
 }

-name = (char *) virURIFormat (tmpuri);
+name = virURIFormat(tmpuri);

 #ifdef HAVE_XMLURI_QUERY_RAW
 VIR_FREE(tmpuri.query_raw);


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



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

Re: [libvirt] [PATCH v5 0/7] Console corruption patchset

2012-02-24 Thread Eric Blake
On 02/23/2012 07:03 AM, Peter Krempa wrote:
 Yet another spin of the console corruption patches.
 
 Current state:
 
 * 1/7 - pidfile: Make checking binary path in virPidFileRead optional
 - No changes to v4.
 - ACKed by Eric
 * 2/7 - Add flags for virDomainOpenConsole
 - No changes to v4.
 - ACKed by Eric
 * 3/7 - virsh: add support for VIR_DOMAIN_CONSOLE_* flags
 - No changes to v4.
 - ACKed by Eric
 * 4/7 - fdstream: Emit stream abort callback even if poll() doesnt.
 - Tweaked according to Eric's review
 * 5/7 - fdstream: Add internal callback on stream close
 - Tweaked according to Eric's review
 * 6/7 - util: Add helpers for safe domain console operations
 - Tweaked according to Eric's review
 * 7/7 - qemu: Add ability to abort existing console while creating new one
 - No changes to v4.
 - ACKed by Eric

ACK series if you make some changes to 6/7.  At this point, I don't know
if we're going to get a timely review from Dan, so I'm comfortable
pushing into the tree now to lengthen the testing time we can give this
prior to the 0.9.11 freeze.

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



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

Re: [libvirt] [PATCH v5 6/7] util: Add helpers for safe domain console operations

2012-02-24 Thread Eric Blake
On 02/23/2012 07:03 AM, Peter Krempa wrote:
 This patch adds a set of functions used in creating console streams for
 domains using PTYs and ensures mutually exclusive access to the PTYs.
 
 If mutualy exclusive access is not used, two clients may open the same

s/mutualy/mutually/

 console, which results in corruption on both clients as both of them
 race to read data from the PTY.
 

 Diff to v4:
 - fixed typos (traditionally)
 - fixed configure.ac to work with automaticaly used values
 - tweaked configure output line to line up with others without
   moving them
 - using STRSKIP to skip the start of the string instead of 
   possibly skipping in the middle of a string
 - fixed whitespace problems
 - changed data type for pids to pid_t and tweaked printing formatters
 - On failure to initialise mutex in virConsoleAlloc don't skip
   to virConsoleFree
 - added comment to clarify why it's needed to unregister the callback
   handler
 - Added OOM error reporting on some error paths.

Looks better.

 
 Note to v4 review:
 I changed the default value for the path of the lock files to auto so it 
 will
 get built with /var/lock on linux machines, so no change to the spec file is
 needed.

auto should default to 'no' rather than error out on systems where we
don't know.

 +dnl UUCP style file locks for PTY consoles
 +if test $with_console_lock_files != no; then
 +  if test $with_console_lock_files = yes; then
 +AC_MSG_ERROR([console lock files requested, but no path given])
 +  elif test $with_console_lock_files = auto; then
 +dnl Default locations for platforms
 +if test $with_linux = yes; then
 +  with_console_lock_files=/var/lock
 +fi
 +  fi
 +  if test $with_console_lock_files = auto; then
 +AC_MSG_ERROR([You must specify path for the lock files on this platform])
 +  fi

I'd write this hunk as:

if test $with_console_lock_files != no; then
  case $with_console_lock_files in
  yes | auto)
dnl Default locations for platforms, or disable if unknown
if test $with_linux = yes; then
  with_console_lock_files=/var/lock
elif test $with_console_lock_files = auto
  with_console_lock_files=no
fi;;
  esac
  if test $with_console_lock_files = yes; then
AC_MSG_ERROR([You must specify path for the lock files on this
platform])
  fi

 +int virConsoleOpen(virConsolesPtr cons,
 +   const char *pty,
 +   virStreamPtr st,
 +   bool force)
 +{
 +virConsoleStreamInfoPtr cbdata = NULL;
 +virStreamPtr savedStream;
 +int ret;
 +
 +virMutexLock(cons-lock);
 +
 +if ((savedStream = virHashLookup(cons-hash, pty))) {
 +if (!force) {
 + /* entry found, console is busy */
 +virMutexUnlock(cons-lock);
 +return 1;
 +   } else {
 +   /* terminate existing connection */
 +   /* The internal close callback handler needs to lock cons-lock to
 +* remove the aborted stream from the hash. This would cause a
 +* deadlock as we would try to enter the lock twice from the very
 +* same thread. We need to unregister the callback and abort the
 +* stream manualy before we create a new console connection.

s/manualy/manually/

ACK with those changes.

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



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

Re: [libvirt] [PATCH v2 3/3] qemu: Implement virDomainPMWakeup API

2012-02-24 Thread Eric Blake
On 02/23/2012 01:44 AM, Michal Privoznik wrote:
 On 15.02.2012 16:04, Michal Privoznik wrote:
 using 'system-wakeup' monitor command. It is supported only in JSON,
 as we are enabling it if possible. Moreover, this command is available
 in qemu-1.1+ which definitely has JSON.
 ---
  src/qemu/qemu_driver.c   |   55 
 ++
  src/qemu/qemu_monitor.c  |   19 ++
  src/qemu/qemu_monitor.h  |2 +
  src/qemu/qemu_monitor_json.c |   21 
  src/qemu/qemu_monitor_json.h |2 +
  5 files changed, 99 insertions(+), 0 deletions(-)

 
 Ping? Eric, it seems to me like you've forgotten this last patch.

Indeed, it fell off my stack of most-recently-pinged patches.  Reviewing
now, and thanks for the ping...

 using 'system-wakeup' monitor command. It is supported only in JSON,
 as we are enabling it if possible. Moreover, this command is available
 in qemu-1.1+ which definitely has JSON.
 ---
  src/qemu/qemu_driver.c   |   55 
 ++
  src/qemu/qemu_monitor.c  |   19 ++
  src/qemu/qemu_monitor.h  |2 +
  src/qemu/qemu_monitor_json.c |   21 
  src/qemu/qemu_monitor_json.h |2 +
  5 files changed, 99 insertions(+), 0 deletions(-)
 
  
 +static int qemuDomainPMWakeup(virDomainPtr dom,
 +  unsigned int flags)

Style nit - we aren't very consistent on whether function names begin on
line 1, but qemu_driver tends to use:

static int
qemuDomainPMWakeup(virDomainPtr dom,
   unsigned int flags)

 +++ b/src/qemu/qemu_monitor_json.c
 @@ -3492,3 +3492,24 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr 
 mon,
  virJSONValueFree(result);
  return ret;
  }
 +
 +int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon)
 +{
 +int ret = -1;
 +virJSONValuePtr cmd = NULL;
 +virJSONValuePtr reply = NULL;
 +
 +cmd = qemuMonitorJSONMakeCommand(system_wakeup, NULL);

Seems so simple :)

ACK.

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



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

[libvirt] [PATCH] build: update to latest gnulib

2012-02-24 Thread Eric Blake
It's been a while, and we're between releases, so now's as good
a time as any to resync.  I didn't notice any showstopper bugs
being fixed, but we definitely get some improvements, such as
tighter syntax checks.

* .gnulib: Update to latest.
* bootstrap: Resync.
* cfg.mk (sc_prohibit_strncmp): Copy upstream changes to
sc_prohibit_strcmp.
---

* .gnulib e9e8aba...be29134 (47):
   stdalign: @samp - @code in doc for consistency
   stdnoreturn: new module
   regex: fix false multibyte matches in some regular expressions
   maint.mk: tell sc_prohibit_strcmp to ding 0 == strcmp (...), too
   streq: Rename macro.
   regex: fix typo in definition of MIN
   acl: Don't use ACL_CNT and similar ops, since they are unreliable.
   acl: Don't use GETACLCNT and similar ops, since they are unreliable.
   acl: Fix endless loop on Solaris with vxfs.
   acl: Fix copy-acl test failure on Solaris 11 2011-11.
   acl: Update doc references.
   Fix test failure in many locales on Solaris 11.
   gnulib-tool: Improve usage message.
   autoupdate
   README-release: make it easier to execute commands
   GNUmakefile: simplify detection of unconfigured trees
   autoupdate
   autoupdate
   autoupdate
   gnulib-tool: Doc fix.
   bootstrap: don't exit 0 upon gnulib-tool failure
   README-release: various improvements
   autoupdate
   maint: replace FSF snail-mail addresses with URLs
   README-release: capitalize a word and split a line
   fatal-signal: use C prototypes (with explicit void).
   regex: spelling fix
   regex: rely on stdint.h for SIZE_MAX
   regex: merge glibc changes
   maint.mk: also prohibit lower-case @var@
   autoupdate
   maint: spelling fixes
   canonicalize: avoid uninitialized memory use
   isatty: Fix test failure of ptsname_r on native Windows.
   spawn-pipe tests: Fix a NULL program name in a diagnostic.
   nonblocking-socket tests: Fix a NULL program name in a diagnostic.
   nonblocking-pipe tests: Fix a NULL program name in a diagnostic.
   canonicalize-lgpl: fix // handling
   canonicalize: fix // handling
   ioctl: Fix test failure on native Windows.
   fsync: Avoid test failure on native Windows.
   * lib/sys_select.in.h [OpenBSD]: When /usr/include/pthread.h is
   sys_select: Avoid syntax error on OpenBSD 5.0.
   get-rusage-as, get-rusage-data tests: Avoid test failure with gcc-4.7.
   stdioext: Fix last commit.
   stdioext: Add tentative support for Plan9.
   file-has-acl: suppress a warning from gcc -Wsuggest-attribute=const

 .gnulib   |2 +-
 bootstrap |6 +++---
 cfg.mk|3 ++-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/.gnulib b/.gnulib
index e9e8aba..be29134 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit e9e8aba12af3c903edd422fa036a356c5b2f313a
+Subproject commit be29134ddd011e6bcf1d73b4ae3d7ee7da60276f
diff --git a/bootstrap b/bootstrap
index 6910abf..31eb651 100755
--- a/bootstrap
+++ b/bootstrap
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Print a version string.
-scriptversion=2012-01-21.16; # UTC
+scriptversion=2012-02-11.09; # UTC

 # Bootstrap this package from checked-out sources.

@@ -423,7 +423,7 @@ check_versions() {
   $use_git || continue
 fi
 # Honor $APP variables ($TAR, $AUTOCONF, etc.)
-appvar=`echo $app | tr '[a-z]-' '[A-Z]_'`
+appvar=`echo $app | LC_ALL=C tr '[a-z]-' '[A-Z]_'`
 test $appvar = TAR  appvar=AMTAR
 case $appvar in
 GZIP) ;; # Do not use $GZIP:  it contains gzip options.
@@ -604,7 +604,7 @@ if $bootstrap_sync; then
 fi

 gnulib_tool=$GNULIB_SRCDIR/gnulib-tool
-$gnulib_tool || exit
+$gnulib_tool || exit $?

 # Get translations.

diff --git a/cfg.mk b/cfg.mk
index 9759d87..ac6c527 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -346,8 +346,9 @@ sc_prohibit_access_xok:

 # Similar to the gnulib maint.mk rule for sc_prohibit_strcmp
 # Use STREQLEN or STRPREFIX rather than comparing strncmp == 0, or != 0.
+snp_ = strncmp *\(.+\)
 sc_prohibit_strncmp:
-   @grep -nE '! *str''ncmp *\(|\str''ncmp *\(.+\) *[!=]=' \
+   @grep -nE '! *strncmp *\(|\$(snp_) *[!=]=|[!=]= *$(snp_)'  \
$$($(VC_LIST_EXCEPT))   \
  | grep -vE ':# *define STR(N?EQLEN|PREFIX)\('   \
  { echo '$(ME): use STREQLEN or STRPREFIX instead of str''ncmp' \
-- 
1.7.7.6

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


[libvirt] [PATCH] qemu: unescape HMP commands before converting them to json

2012-02-24 Thread Josh Durgin
QMP commands don't need to be escaped since converting them to json
also escapes special characters. When a QMP command fails, however,
libvirt falls back to HMP commands. These fallback functions
(qemuMonitorText*) do their own escaping, and pass the result directly
to qemuMonitorHMPCommandWithFd. If the monitor is in json mode, these
pre-escaped commands will be escaped again when converted to json,
which can result in the wrong arguments being sent.

For example, a filename test\file would be sent in json as
test\\file.

This prevented attaching an image file with a  or \ in its name in
qemu 1.0.50, and also broke rbd attachment (which uses backslashes to
escape some internal arguments.)

Reported-by: Masuko Tomoya tomoya.mas...@gmail.com
Signed-off-by: Josh Durgin josh.dur...@dreamhost.com
---
 .gitignore  |1 +
 src/qemu/qemu_monitor.c |   59 +++-
 src/qemu/qemu_monitor.h |1 +
 tests/Makefile.am   |   12 -
 tests/qemumonitortest.c |  114 +++
 5 files changed, 181 insertions(+), 6 deletions(-)
 create mode 100644 tests/qemumonitortest.c

diff --git a/.gitignore b/.gitignore
index b7561dc..264a419 100644
--- a/.gitignore
+++ b/.gitignore
@@ -128,6 +128,7 @@
 /tests/openvzutilstest
 /tests/qemuargv2xmltest
 /tests/qemuhelptest
+/tests/qemumonitortest
 /tests/qemuxmlnstest
 /tests/qparamtest
 /tests/reconnect
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 93f3505..85212a7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -153,6 +153,49 @@ char *qemuMonitorEscapeArg(const char *in)
 return out;
 }
 
+char *qemuMonitorUnescapeArg(const char *in)
+{
+int i, j;
+char *out;
+int len = strlen(in) + 1;
+char next;
+
+if (VIR_ALLOC_N(out, len)  0)
+return NULL;
+
+for (i = j = 0; i  len; ++i) {
+next = in[i];
+if (in[i] == '\\') {
+if (len  i + 1) {
+// trailing backslash shouldn't be possible
+VIR_FREE(out);
+return NULL;
+}
+++i;
+switch(in[i]) {
+case 'r':
+next = '\r';
+break;
+case 'n':
+next = '\n';
+break;
+case '':
+case '\\':
+next = in[i];
+break;
+default:
+// invalid input
+VIR_FREE(out);
+return NULL;
+}
+}
+out[j++] = next;
+}
+out[j] = '\0';
+
+return out;
+}
+
 #if DEBUG_RAW_IO
 # include c-ctype.h
 static char * qemuMonitorEscapeNonPrintable(const char *text)
@@ -852,10 +895,20 @@ int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
 int scm_fd,
 char **reply)
 {
-if (mon-json)
-return qemuMonitorJSONHumanCommandWithFd(mon, cmd, scm_fd, reply);
-else
+char *json_cmd = NULL;
+if (mon-json) {
+// hack to avoid complicating each call to text monitor functions
+json_cmd = qemuMonitorUnescapeArg(cmd);
+if (!json_cmd) {
+VIR_DEBUG(Could not unescape command: %s, cmd);
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(Unable to unescape command));
+return -1;
+}
+return qemuMonitorJSONHumanCommandWithFd(mon, json_cmd, scm_fd, reply);
+} else {
 return qemuMonitorTextCommandWithFd(mon, cmd, scm_fd, reply);
+}
 }
 
 /* Ensure proper locking around callbacks.  */
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 7c6c52b..9768457 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -128,6 +128,7 @@ struct _qemuMonitorCallbacks {
 
 
 char *qemuMonitorEscapeArg(const char *in);
+char *qemuMonitorUnescapeArg(const char *in);
 
 qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm,
virDomainChrSourceDefPtr config,
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9974c2f..3e505a5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -72,6 +72,7 @@ EXTRA_DIST =  \
nwfilterxml2xmlout \
oomtrace.pl \
qemuhelpdata \
+   qemumonitortest \
qemuxml2argvdata \
qemuxml2xmloutdata \
qemuxmlnsdata \
@@ -110,7 +111,8 @@ check_PROGRAMS += xml2sexprtest sexpr2xmltest \
 endif
 if WITH_QEMU
 check_PROGRAMS += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \
-   qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest
+   qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \
+   qemumonitortest
 endif
 
 if WITH_OPENVZ
@@ -237,7 +239,8 @@ endif
 
 if WITH_QEMU
 TESTS += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest qemuargv2xmltest \
-qemuhelptest domainsnapshotxml2xmltest nwfilterxml2xmltest
+qemuhelptest domainsnapshotxml2xmltest 

[libvirt] Add GSoC project ideas to the wiki!

2012-02-24 Thread Stefan Hajnoczi
This is a reminder that QEMU will apply for Google Summer of Code 2012 and we
need project ideas and mentors.  Libvirt and kvm.ko projects are also welcome!

http://wiki.qemu.org/Google_Summer_of_Code_2012

Please add yourself to the wiki now if you want to mentor a project
this summer.  I will file our application next week.

Thanks,
Stefan

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


[libvirt] [PATCH v4] Support for cpu64-rhel* qemu cpu models

2012-02-24 Thread Martin Kletzander
In qemu there are 2 cpu models (cpu64-rhel5 and cpu64-rhel6) not
supported by libvirt. This patch adds the support with the flags
specifications from /usr/share/qemu-kvm/cpu-model/cpu-x86_64.conf
The only difference is that AMD-specific features are removed so
the processor type is not vendor-specific.
---
v4:
 - removed AMD-specific features (3dnow, 3dnowext, svm)

v3:
 - fixed sse3 naming (it's 'pni' in the features)

v2:
 - removed duplicated entries

 src/cpu/cpu_map.xml |   60 +++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 2053f96..da3db02 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -309,6 +309,66 @@
   feature name='pni'/
 /model

+model name='cpu64-rhel5'
+  feature name='apic'/
+  feature name='clflush'/
+  feature name='cmov'/
+  feature name='cx8'/
+  feature name='de'/
+  feature name='fpu'/
+  feature name='fxsr'/
+  feature name='lm'/
+  feature name='mca'/
+  feature name='mce'/
+  feature name='mmx'/
+  feature name='msr'/
+  feature name='mtrr'/
+  feature name='nx'/
+  feature name='pae'/
+  feature name='pat'/
+  feature name='pge'/
+  feature name='pse'/
+  feature name='pse36'/
+  feature name='sep'/
+  feature name='sse'/
+  feature name='sse2'/
+  feature name='pni'/
+  feature name='syscall'/
+  feature name='tsc'/
+/model
+
+model name='cpu64-rhel6'
+  feature name='abm'/
+  feature name='apic'/
+  feature name='clflush'/
+  feature name='cmov'/
+  feature name='cx16'/
+  feature name='cx8'/
+  feature name='de'/
+  feature name='fpu'/
+  feature name='fxsr'/
+  feature name='lahf_lm'/
+  feature name='lm'/
+  feature name='mca'/
+  feature name='mce'/
+  feature name='mmx'/
+  feature name='msr'/
+  feature name='mtrr'/
+  feature name='nx'/
+  feature name='pae'/
+  feature name='pat'/
+  feature name='pge'/
+  feature name='pse'/
+  feature name='pse36'/
+  feature name='sep'/
+  feature name='sse'/
+  feature name='sse2'/
+  feature name='pni'/
+  feature name='sse4a'/
+  feature name='syscall'/
+  feature name='tsc'/
+/model
+
 model name='kvm32'
   model name='pentiumpro'/
   feature name='mtrr'/
--
1.7.3.4

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


[libvirt] [PATCHv2] Error out when using SPICE TLS with spice_tls=0

2012-02-24 Thread Christophe Fergeau
It's possible to disable SPICE TLS in qemu.conf. When this happens,
libvirt ignores any SPICE TLS port or x509 directory that may have
been set when it builds the qemu command line to use. However, it's
not ignoring the secure channels that may have been set and adds
tls-channel arguments to qemu command line.
Current qemu versions don't report an error when this happens, and try to use
TLS for the specified channels.

Before this patch

domain type='kvm'
  nameauto-tls-port/name
  memory65536/memory
  os
type arch='x86_64' machine='pc'hvm/type
  /os
  devices
graphics type='spice' port='5900' tlsPort='-1' autoport='yes' listen='0' ke
  listen type='address' address='0'/
  channel name='main' mode='secure'/
  channel name='inputs' mode='secure'/
/graphics
  /devices
/domain

generates

-spice port=5900,addr=0,disable-ticketing,tls-channel=main,tls-channel=inputs

and starts QEMU.

After this patch, an error is reported if a TLS port is set in the XML
or if secure channels are specified but TLS is disabled in qemu.conf.
This is the behaviour the oVirt people (where I spotted this issue) said
they would expect.

This fixes bug #790436
---
 src/qemu/qemu_command.c |   12 +++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5a34504..4f3e61e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5231,7 +5231,12 @@ qemuBuildCommandLine(virConnectPtr conn,
 
 virBufferAsprintf(opt, port=%u, def-graphics[0]-data.spice.port);
 
-if (driver-spiceTLS  def-graphics[0]-data.spice.tlsPort != -1)
+if (def-graphics[0]-data.spice.tlsPort != -1)
+if (!driver-spiceTLS) {
+qemuReportError(VIR_ERR_XML_ERROR,
+_(spice TLS port set in XML configuration, 
but TLS is disabled in qemu.conf));
+goto error;
+}
 virBufferAsprintf(opt, ,tls-port=%u, 
def-graphics[0]-data.spice.tlsPort);
 
 switch (virDomainGraphicsListenGetType(def-graphics[0], 0)) {
@@ -5287,6 +5292,11 @@ qemuBuildCommandLine(virConnectPtr conn,
 int mode = def-graphics[0]-data.spice.channels[i];
 switch (mode) {
 case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
+if (!driver-spiceTLS) {
+qemuReportError(VIR_ERR_XML_ERROR,
+_(spice secure channels set in XML 
configuration, but TLS is disabled in qemu.conf));
+goto error;
+}
 virBufferAsprintf(opt, ,tls-channel=%s,
   
virDomainGraphicsSpiceChannelNameTypeToString(i));
 break;
-- 
1.7.7.6

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


Re: [libvirt] Add GSoC project ideas to the wiki!

2012-02-24 Thread Michal Privoznik
On 24.02.2012 10:19, Stefan Hajnoczi wrote:
 This is a reminder that QEMU will apply for Google Summer of Code 2012 and we
 need project ideas and mentors.  Libvirt and kvm.ko projects are also welcome!
 
 http://wiki.qemu.org/Google_Summer_of_Code_2012
 
 Please add yourself to the wiki now if you want to mentor a project
 this summer.  I will file our application next week.
 
 Thanks,
 Stefan

Hi Stefan,

Thank you for the opportunity. I was personally thinking about something
libvirt-snmp related. Nowdays, it is difficult to add new elements to
MIB, as some parts of code were generated by mib2c. Any change to MIB
requires regeneration of such source files and thus leads to loss of all
previous changes. So one thing that is coming to my mind is drop this
dependency and use libsnmp directly. But I am not sure it is worth of GSoC.

Michal

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


Re: [libvirt] [PATCHv2] Error out when using SPICE TLS with spice_tls=0

2012-02-24 Thread Daniel P. Berrange
On Fri, Feb 24, 2012 at 11:34:45AM +0100, Christophe Fergeau wrote:
 It's possible to disable SPICE TLS in qemu.conf. When this happens,
 libvirt ignores any SPICE TLS port or x509 directory that may have
 been set when it builds the qemu command line to use. However, it's
 not ignoring the secure channels that may have been set and adds
 tls-channel arguments to qemu command line.
 Current qemu versions don't report an error when this happens, and try to use
 TLS for the specified channels.
 
 Before this patch
 
 domain type='kvm'
   nameauto-tls-port/name
   memory65536/memory
   os
 type arch='x86_64' machine='pc'hvm/type
   /os
   devices
 graphics type='spice' port='5900' tlsPort='-1' autoport='yes' listen='0' 
 ke
   listen type='address' address='0'/
   channel name='main' mode='secure'/
   channel name='inputs' mode='secure'/
 /graphics
   /devices
 /domain
 
 generates
 
 -spice port=5900,addr=0,disable-ticketing,tls-channel=main,tls-channel=inputs
 
 and starts QEMU.
 
 After this patch, an error is reported if a TLS port is set in the XML
 or if secure channels are specified but TLS is disabled in qemu.conf.
 This is the behaviour the oVirt people (where I spotted this issue) said
 they would expect.
 
 This fixes bug #790436
 ---
  src/qemu/qemu_command.c |   12 +++-
  1 files changed, 11 insertions(+), 1 deletions(-)
 
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 5a34504..4f3e61e 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -5231,7 +5231,12 @@ qemuBuildCommandLine(virConnectPtr conn,
  
  virBufferAsprintf(opt, port=%u, 
 def-graphics[0]-data.spice.port);
  
 -if (driver-spiceTLS  def-graphics[0]-data.spice.tlsPort != -1)
 +if (def-graphics[0]-data.spice.tlsPort != -1)
 +if (!driver-spiceTLS) {
 +qemuReportError(VIR_ERR_XML_ERROR,
 +_(spice TLS port set in XML configuration, 
 but TLS is disabled in qemu.conf));
 +goto error;
 +}
  virBufferAsprintf(opt, ,tls-port=%u, 
 def-graphics[0]-data.spice.tlsPort);
  
  switch (virDomainGraphicsListenGetType(def-graphics[0], 0)) {
 @@ -5287,6 +5292,11 @@ qemuBuildCommandLine(virConnectPtr conn,
  int mode = def-graphics[0]-data.spice.channels[i];
  switch (mode) {
  case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
 +if (!driver-spiceTLS) {
 +qemuReportError(VIR_ERR_XML_ERROR,
 +_(spice secure channels set in XML 
 configuration, but TLS is disabled in qemu.conf));
 +goto error;
 +}
  virBufferAsprintf(opt, ,tls-channel=%s,

 virDomainGraphicsSpiceChannelNameTypeToString(i));
  break;

ACK, if we  s/XML_ERROR/CONFIG_UNSUPPORTED/  in the those two error
messages

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


[libvirt] [PATCH] Fixed service handling in specfile

2012-02-24 Thread Martin Kletzander
After adding the libvirt-guests service into usual runlevels, we used
to start the libvirt-guests service. However this is usually not a
good practice. As mentioned on fedoraproject wiki, the installations
can be in changeroots, in an installer context, or in other situations
where we don't want the services autostarted.
---
 libvirt.spec.in |8 
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 67cde23..072fd8e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1027,14 +1027,6 @@ fi
 %if %{with_systemd}
 %else
 /sbin/chkconfig --add libvirt-guests
-if [ $1 -ge 1 ]; then
-level=$(/sbin/runlevel | /bin/cut -d ' ' -f 2)
-if /sbin/chkconfig --levels $level libvirt-guests; then
-# this doesn't do anything but allowing for libvirt-guests to be
-# stopped on the first shutdown
-/sbin/service libvirt-guests start  /dev/null 21 || true
-fi
-fi
 %endif

 %postun client -p /sbin/ldconfig
-- 
1.7.3.4

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


Re: [libvirt] libvirt TCK wrapper for autotest review

2012-02-24 Thread Lucas Meneghel Rodrigues

On 02/23/2012 12:27 PM, Guannan Ren wrote:


Hi Lucas,

Thanks for your these good modifications.
There is one place I noticed where you output each testcase of *.t
into a separate file with .tap extension.
hence, it has a corresponding log file with little content for each
testcase. it seem a little harder to check compared
to just one log file.
The rest of them is perfect for me.

Guannan Ren



Thanks!

Now, feel free to pick this up and modify as you wish, then send me as a 
pull request/patch to the mailing list. I'll close the example pull 
request and will be waiting on you, ok?


Cheers,

Lucas

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


Re: [libvirt] [Patch v2] vmx: Better Workstation vmx handling

2012-02-24 Thread Matthias Bolte
2012/2/23 Jean-Baptiste Rouault jean-baptiste.roua...@diateam.net:
 This patch adds support for vmx files with empty networkName
 values (which is the case for vmx generated by Workstation).
 It also adds support for vmx containing NATed network interfaces.

 Update test suite accordingly
 ---

ACK and pushed, thanks.

-- 
Matthias Bolte
http://photron.blogspot.com

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


Re: [libvirt] Add GSoC project ideas to the wiki!

2012-02-24 Thread Stefan Hajnoczi
On Fri, Feb 24, 2012 at 10:38 AM, Michal Privoznik mpriv...@redhat.com wrote:
 On 24.02.2012 10:19, Stefan Hajnoczi wrote:
 Thank you for the opportunity. I was personally thinking about something
 libvirt-snmp related. Nowdays, it is difficult to add new elements to
 MIB, as some parts of code were generated by mib2c. Any change to MIB
 requires regeneration of such source files and thus leads to loss of all
 previous changes. So one thing that is coming to my mind is drop this
 dependency and use libsnmp directly. But I am not sure it is worth of GSoC.

If this project is self-contained and can be completed in 12 weeks by
a person without prior libvirt and SNMP experience, but fluent C
programming skills, then it sounds suitable.

Is there any creativity required or problems to solve that aren't
grunt work?  For example, if you just need to run mib2c and then
manually diff to produce the final C version, then this sounds like a
lot of manual work but little gain for the student.

Stefan

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


Re: [libvirt] RFC: Post-install behavior of libvirt(-client)

2012-02-24 Thread Martin Kletzander
On 02/23/2012 04:23 PM, Eric Blake wrote:
 On 02/23/2012 07:25 AM, Martin Kletzander wrote:

 To me it seems more reasonable to just don't start anything. I don't
 want any service on my system started just because I installed it.
 
 I think we've reached a state of violent agreement :)

I didn't mean it that way but I'm happy we came to an agreement :)


 So to summarize it. Please tell me if I can remove the parts of the
 %post script dealing with starting services.
 
 Yes, and I will gladly ACK such patches.

Patch is sent (it was starting the service on only one place, so it's
really small one).

 Thanks for tackling this, and for making me do further research onto
 Fedora packaging guidelines.

And thank you for having such patience with me :)

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


[libvirt] [PATCH v2] Fixed URI parsing

2012-02-24 Thread Martin Kletzander
Function xmlParseURI does not remove square brackets around IPv6
address when parsing. One of the solutions is making wrappers around
functions working with xmlURI*. This assures that uri-server will be
always properly assigned and it doesn't have to be changed when used
on some new place in the code.
For this purpose, functions virParseURI and virSaveURI were
added. These function are wrappers around xmlParseURI and xmlSaveUri
respectively.

File changes:
 - src/util/xml.h   -- declaration
 - src/util/xml.c   -- definition
 - src/libvirt_private.syms -- symbol export
 - all others   -- function call fixed and include added
---
v2:
 - added virSaveURI for building back the original string

 src/esx/esx_driver.c   |3 +-
 src/libvirt.c  |7 ++-
 src/libvirt_private.syms   |2 +
 src/libxl/libxl_driver.c   |3 +-
 src/lxc/lxc_driver.c   |3 +-
 src/openvz/openvz_driver.c |3 +-
 src/qemu/qemu_driver.c |2 +-
 src/qemu/qemu_migration.c  |5 +-
 src/remote/remote_driver.c |5 +-
 src/uml/uml_driver.c   |3 +-
 src/util/xml.c |   94 
 src/util/xml.h |5 ++
 src/vbox/vbox_tmpl.c   |3 +-
 src/vmx/vmx.c  |3 +-
 src/xen/xen_driver.c   |2 +-
 src/xen/xend_internal.c|3 +-
 16 files changed, 129 insertions(+), 17 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f5e1cc7..4375432 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -44,6 +44,7 @@
 #include esx_vi.h
 #include esx_vi_methods.h
 #include esx_util.h
+#include xml.h

 #define VIR_FROM_THIS VIR_FROM_ESX

@@ -3976,7 +3977,7 @@ esxDomainMigratePerform(virDomainPtr domain,
 }

 /* Parse migration URI */
-parsedUri = xmlParseURI(uri);
+parsedUri = virParseURI(uri);

 if (parsedUri == NULL) {
 virReportOOMError();
diff --git a/src/libvirt.c b/src/libvirt.c
index 6294196..465d0aa 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -44,6 +44,7 @@
 #include command.h
 #include virnodesuspend.h
 #include virrandom.h
+#include xml.h

 #ifndef WITH_DRIVER_MODULES
 # ifdef WITH_TEST
@@ -1127,7 +1128,7 @@ do_open (const char *name,
 virConnectOpenResolveURIAlias(name, alias)  0)
 goto failed;

-ret-uri = xmlParseURI (alias ? alias : name);
+ret-uri = virParseURI (alias ? alias : name);
 if (!ret-uri) {
 virLibConnError(VIR_ERR_INVALID_ARG,
 _(could not parse connection URI %s),
@@ -1729,7 +1730,7 @@ virConnectGetURI (virConnectPtr conn)
 return NULL;
 }

-name = (char *)xmlSaveUri(conn-uri);
+name = (char *)virSaveURI(conn-uri);
 if (!name) {
 virReportOOMError();
 goto error;
@@ -4964,7 +4965,7 @@ virDomainMigratePeer2Peer (virDomainPtr domain,
 return -1;
 }

-tempuri = xmlParseURI(dconnuri);
+tempuri = virParseURI(dconnuri);
 if (!tempuri) {
 virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
 virDispatchError(domain-conn);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9e3573f..595020a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1431,6 +1431,8 @@ virTypedParameterAssign;


 # xml.h
+virParseURI;
+virSaveURI;
 virXMLChildElementCount;
 virXMLParseHelper;
 virXMLPropString;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 6cfc5eb..f1ef5fb 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -44,6 +44,7 @@
 #include libxl_conf.h
 #include xen_xm.h
 #include virtypedparam.h
+#include xml.h

 #define VIR_FROM_THIS VIR_FROM_LIBXL

@@ -1043,7 +1044,7 @@ libxlOpen(virConnectPtr conn,
 if (libxl_driver == NULL)
 return VIR_DRV_OPEN_DECLINED;

-conn-uri = xmlParseURI(xen:///);
+conn-uri = virParseURI(xen:///);
 if (!conn-uri) {
 virReportOOMError();
 return VIR_DRV_OPEN_ERROR;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index b6962cf7..3d25d5e 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -60,6 +60,7 @@
 #include virnodesuspend.h
 #include virtime.h
 #include virtypedparam.h
+#include xml.h

 #define VIR_FROM_THIS VIR_FROM_LXC

@@ -139,7 +140,7 @@ static virDrvOpenStatus lxcOpen(virConnectPtr conn,
 if (lxc_driver == NULL)
 return VIR_DRV_OPEN_DECLINED;

-conn-uri = xmlParseURI(lxc:///);
+conn-uri = virParseURI(lxc:///);
 if (!conn-uri) {
 virReportOOMError();
 return VIR_DRV_OPEN_ERROR;
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 833a98d..47fad74 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -56,6 +56,7 @@
 #include virfile.h
 #include logging.h
 #include command.h
+#include xml.h

 #define VIR_FROM_THIS VIR_FROM_OPENVZ

@@ -1335,7 

Re: [libvirt] [PATCH v2] Fixed URI parsing

2012-02-24 Thread Daniel P. Berrange
On Fri, Feb 24, 2012 at 02:30:11PM +0100, Martin Kletzander wrote:
 Function xmlParseURI does not remove square brackets around IPv6
 address when parsing. One of the solutions is making wrappers around
 functions working with xmlURI*. This assures that uri-server will be
 always properly assigned and it doesn't have to be changed when used
 on some new place in the code.
 For this purpose, functions virParseURI and virSaveURI were
 added. These function are wrappers around xmlParseURI and xmlSaveUri
 respectively.


 diff --git a/src/util/xml.h b/src/util/xml.h
 index a3750fa..4835900 100644
 --- a/src/util/xml.h
 +++ b/src/util/xml.h
 @@ -10,6 +10,7 @@
  # include libxml/parser.h
  # include libxml/tree.h
  # include libxml/xpath.h
 +# include libxml/uri.h
 
  int  virXPathBoolean(const char *xpath,
   xmlXPathContextPtr ctxt);
 @@ -61,6 +62,10 @@ xmlDocPtr  virXMLParseHelper(int domcode,
   const char *url,
   xmlXPathContextPtr *pctxt);
 
 +xmlURIPtrvirParseURI(const char *uri);
 +
 +unsigned char *   virSaveURI(xmlURIPtr uri);
 +

Can you create new files for these  'util/viruri.{h,c}' and change
to ensure a standard 'virURI' naming prefix. Also we tend to use
the pair  Parse/Format, rather than Parse/Save in libvirt code.

So eg create a file viruri.h containing:

   typedef virURIPtr xmlURIPtr;

   virURIPtr  virURIParse(const char *uri);
   char * virURIFormat(virURIPtr uri);

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 v2] Fixed URI parsing

2012-02-24 Thread Eric Blake
On 02/24/2012 06:30 AM, Martin Kletzander wrote:
 Function xmlParseURI does not remove square brackets around IPv6
 address when parsing. One of the solutions is making wrappers around
 functions working with xmlURI*. This assures that uri-server will be
 always properly assigned and it doesn't have to be changed when used
 on some new place in the code.
 For this purpose, functions virParseURI and virSaveURI were
 added. These function are wrappers around xmlParseURI and xmlSaveUri
 respectively.
 

 + */
 +unsigned char *
 +virSaveURI(xmlURIPtr uri)
 +{
 +char *tmpserver, *backupserver = uri-server;
 +unsigned char *ret;
 +bool fixback = false;

Instead of using bool fixback, I'd set tmpserver as NULL here...

 +
 +/* First check: does it make sense to do anything */
 +if (uri != NULL 
 +uri-server != NULL 
 +strchr(uri-server, ':') != NULL) {
 +
 +/* To be sure we have enough space for the brackets, we save
 + * the old string and then alocate a new one */

s/alocate/allocate/ - but see below, as I don't think you need this comment.

 +
 + /* the + 2 is room for square brackets and + 1 for '\0' */
 +size_t length = strlen(uri-server) + 3;
 +
 +if(VIR_ALLOC_N(tmpserver, length)  0) {
 +virReportOOMError();

No need to raise OOM error here - all callers of xmlSaveUri were already
raising their own OOM after a NULL return.

 +return NULL;
 +}
 +
 +snprintf(tmpserver, length, [%s], uri-server);

I'd just use virAsnprintf(tmpserver, [%s], uri-server); none of this
need to call strlen, VIR_ALLOC_N, or snprintf.

 +
 +uri-server = tmpserver;
 +bool fixback = true;
 +}
 +
 +ret = xmlSaveUri(uri);
 +
 +/* Put the fixed version back */
 +if (fixback) {

...and check 'if (tmpserver)' here (that is, fixback is redundant with
whether tmpserver was assigned at this point).

 +uri-server = backupserver;
 +VIR_FREE(tmpserver);
 +}
 +
 +return ret;
 +}
 

I'm also a bit worried that we might regress if we don't add a syntax
check; can you look at adding a rule to cfg.mk that poisons xmlParseURI
and xmlSaveUri (wow, libxml2 really does have the awful URI vs. Uri in
its naming conventions), then exempt util/uri.c from the check as the
only file allowed to use them.

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



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