Re: [libvirt] [Patch]: spice agent-mouse support

2012-02-29 Thread Osier Yang

On 03/01/2012 11:54 AM, Zhou Peng wrote:

Signed-off-by: Zhou Peng

spice agent-mouse support

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f9654f1..79d5ac9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -852,6 +852,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
  break;

  case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+VIR_FREE(def->data.spice.agentmouse);
  VIR_FREE(def->data.spice.keymap);
  virDomainGraphicsAuthDefClear(&def->data.spice.auth);
  break;
@@ -5543,6 +5544,8 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
  VIR_FREE(autoport);
  }

+def->data.spice.agentmouse = virXMLPropString(node, "agentmouse");
+
  def->data.spice.keymap = virXMLPropString(node, "keymap");

  if (virDomainGraphicsAuthDefParseXML(node,&def->data.spice.auth,
@@ -11364,6 +11367,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
  if (listenAddr)
  virBufferAsprintf(buf, " listen='%s'", listenAddr);

+if (def->data.spice.agentmouse)
+virBufferEscapeString(buf, " agentmouse='%s'",
+  def->data.spice.agentmouse);
+
  if (def->data.spice.keymap)
  virBufferEscapeString(buf, " keymap='%s'",
def->data.spice.keymap);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 596be4d..e55995c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1072,6 +1072,7 @@ struct _virDomainGraphicsDef {
  struct {
  int port;
  int tlsPort;
+char *agentmouse;
  char *keymap;
  virDomainGraphicsAuthDef auth;
  unsigned int autoport :1;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 01adf0d..531ecbe 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5391,6 +5391,10 @@ qemuBuildCommandLine(virConnectPtr conn,

  VIR_FREE(netAddr);

+if (def->graphics[0]->data.spice.agentmouse)
+virBufferAsprintf(&opt, ",agent-mouse=%s",
+  def->graphics[0]->data.spice.agentmouse);
+
  /* In the password case we set it via monitor command, to avoid
   * making it visible on CLI, so there's no use of password=XXX
   * in this bit of the code */
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args
b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args
index 681f7c2..746c116 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args
@@ -5,5 +5,5 @@ virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -hda \
  /dev/HostVG/QEMUGuest1 -chardev spicevmc,id=charchannel0,name=vdagent -device 
\
  virtserialport,bus=virtio-serial1.0,nr=3,chardev=charchannel0,id=channel0\
  ,name=com.redhat.spice.0 -usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\
-x509-dir=/etc/pki/libvirt-spice,tls-channel=main -device \
+agent-mouse=off,x509-dir=/etc/pki/libvirt-spice,tls-channel=main -device \
  virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml
b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml
index 6505b55..266a4ed 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml
@@ -23,7 +23,7 @@
  

  
-
+

  
  

--
Zhou Peng


Docmentation && XML schema is necessay, you have to update
docs/formatdomain.html.in and docs/schemas/domaincommon.rng.
Aslo you have to make sure the value for "agentmouse" is
valid ("on|off").

And the new test only tests "agentmouse=off", there should
be one for "agentmount=on" too.

If qemu didn't always support "agentmouse", you have to detect
if qemu supports it, and error out if it's not supported.

Commit 5edfcaae6f7ebb could serve as an example for you.

Osier

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


[libvirt] [libvirt-glib] Remove now redundant 'path' property

2012-02-29 Thread Zeeshan Ali (Khattak)
From: "Zeeshan Ali (Khattak)" 

Remove now redundant 'path' property from GVirDomainDevice subclasses.
These classes now have access to their configurations, from which they
can easily get the path (among other properties) internally.
---
 libvirt-gobject/libvirt-gobject-domain-disk.c  |   84 ---
 libvirt-gobject/libvirt-gobject-domain-interface.c |   85 ---
 2 files changed, 36 insertions(+), 133 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c 
b/libvirt-gobject/libvirt-gobject-domain-disk.c
index fb7672e..0a1493a 100644
--- a/libvirt-gobject/libvirt-gobject-domain-disk.c
+++ b/libvirt-gobject/libvirt-gobject-domain-disk.c
@@ -36,73 +36,25 @@
 
 struct _GVirDomainDiskPrivate
 {
-gchar *path;
+gboolean unused;
 };
 
 G_DEFINE_TYPE(GVirDomainDisk, gvir_domain_disk, GVIR_TYPE_DOMAIN_DEVICE);
 
-enum {
-PROP_0,
-PROP_PATH,
-};
-
 #define GVIR_DOMAIN_DISK_ERROR gvir_domain_disk_error_quark()
 
-
 static GQuark
 gvir_domain_disk_error_quark(void)
 {
 return g_quark_from_static_string("gvir-domain-disk");
 }
 
-static void gvir_domain_disk_get_property(GObject *object,
-  guint prop_id,
-  GValue *value,
-  GParamSpec *pspec)
-{
-GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
-GVirDomainDiskPrivate *priv = self->priv;
-
-switch (prop_id) {
-case PROP_PATH:
-g_value_set_string(value, priv->path);
-break;
-
-default:
-G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
-}
-}
-
-
-static void gvir_domain_disk_set_property(GObject *object,
-  guint prop_id,
-  const GValue *value,
-  GParamSpec *pspec)
-{
-GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
-GVirDomainDiskPrivate *priv = self->priv;
-
-switch (prop_id) {
-case PROP_PATH:
-g_free(priv->path);
-priv->path = g_value_dup_string(value);
-break;
-
-default:
-G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
-}
-}
-
-
 static void gvir_domain_disk_finalize(GObject *object)
 {
 GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
-GVirDomainDiskPrivate *priv = self->priv;
 
 g_debug("Finalize GVirDomainDisk=%p", self);
 
-g_free(priv->path);
-
 G_OBJECT_CLASS(gvir_domain_disk_parent_class)->finalize(object);
 }
 
@@ -111,19 +63,6 @@ static void gvir_domain_disk_class_init(GVirDomainDiskClass 
*klass)
 GObjectClass *object_class = G_OBJECT_CLASS (klass);
 
 object_class->finalize = gvir_domain_disk_finalize;
-object_class->get_property = gvir_domain_disk_get_property;
-object_class->set_property = gvir_domain_disk_set_property;
-
-g_object_class_install_property(object_class,
-PROP_PATH,
-g_param_spec_string("path",
-"Path",
-"The disk path",
-NULL,
-G_PARAM_READWRITE |
-G_PARAM_CONSTRUCT_ONLY 
|
-
G_PARAM_STATIC_STRINGS));
-
 g_type_class_add_private(klass, sizeof(GVirDomainDiskPrivate));
 }
 
@@ -151,6 +90,15 @@ gvir_domain_disk_stats_free(GVirDomainDiskStats *stats)
 G_DEFINE_BOXED_TYPE(GVirDomainDiskStats, gvir_domain_disk_stats,
 gvir_domain_disk_stats_copy, gvir_domain_disk_stats_free)
 
+static gchar *gvir_domain_disk_get_path(GVirDomainDisk *self)
+{
+GVirConfigDomainDevice *config;
+
+config = gvir_domain_device_get_config(GVIR_DOMAIN_DEVICE(self));
+
+return gvir_config_domain_disk_get_target_dev (GVIR_CONFIG_DOMAIN_DISK 
(config));
+}
+
 /**
  * gvir_domain_disk_get_stats:
  * @self: the domain disk
@@ -166,15 +114,15 @@ GVirDomainDiskStats 
*gvir_domain_disk_get_stats(GVirDomainDisk *self, GError **e
 {
 GVirDomainDiskStats *ret = NULL;
 virDomainBlockStatsStruct stats;
-GVirDomainDiskPrivate *priv;
 virDomainPtr handle;
+gchar *path;
 
 g_return_val_if_fail(GVIR_IS_DOMAIN_DISK(self), NULL);
 
-priv = self->priv;
 handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self));
+path = gvir_domain_disk_get_path (self);
 
-if (virDomainBlockStats(handle, priv->path, &stats, sizeof (stats)) < 0) {
+if (virDomainBlockStats(handle, path, &stats, sizeof (stats)) < 0) {
 gvir_set_error_literal(err, GVIR_DOMAIN_DISK_ERROR,
0,
"Unable to get domain disk stats");
@@ -190,6 +138,7 @@ GVirDomainDiskStats 
*gvir_domain_disk_get_stats(

[libvirt] [libvirt-glib] RFC: Empty statistics for user-mode interfaces

2012-02-29 Thread Zeeshan Ali (Khattak)
From: "Zeeshan Ali (Khattak)" 

One of the limitations of user-mode networking of libvirt is that you
can't get statistics for it (not yet, at least). Instead of erroring-out
in that case, simply return empty statistics result and spit a debug
message.

I will merge this into my 'Remove now redundant 'path' property' patch
once someone tells me that this makes sense. :)
---
 libvirt-gobject/libvirt-gobject-domain-interface.c |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.c 
b/libvirt-gobject/libvirt-gobject-domain-interface.c
index 0a9bde0..4436466 100644
--- a/libvirt-gobject/libvirt-gobject-domain-interface.c
+++ b/libvirt-gobject/libvirt-gobject-domain-interface.c
@@ -94,7 +94,11 @@ static gchar 
*gvir_domain_interface_get_path(GVirDomainInterface *self)
 gchar *path = NULL;
 
 config = gvir_domain_device_get_config(GVIR_DOMAIN_DEVICE(self));
-path = 
gvir_config_domain_interface_get_ifname(GVIR_CONFIG_DOMAIN_INTERFACE (config));
+if (GVIR_CONFIG_IS_DOMAIN_INTERFACE_USER(self))
+/* FIXME: One of the limitations of user-mode networking of libvirt */
+g_debug("Statistics gathering for user-mode network not yet 
supported");
+else
+path = 
gvir_config_domain_interface_get_ifname(GVIR_CONFIG_DOMAIN_INTERFACE (config));
 
 g_object_unref (config);
 
@@ -123,6 +127,10 @@ GVirDomainInterfaceStats 
*gvir_domain_interface_get_stats(GVirDomainInterface *s
 
 handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self));
 path = gvir_domain_interface_get_path (self);
+if (path == NULL) {
+ret = g_slice_new0(GVirDomainInterfaceStats);
+goto end;
+}
 
 if (virDomainInterfaceStats(handle, path, &stats, sizeof (stats)) < 0) {
 gvir_set_error_literal(err, GVIR_DOMAIN_INTERFACE_ERROR,
-- 
1.7.7.6

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


[libvirt] [Patch]: spice agent-mouse support

2012-02-29 Thread Zhou Peng
Signed-off-by: Zhou Peng 

spice agent-mouse support

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f9654f1..79d5ac9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -852,6 +852,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
 break;

 case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+    VIR_FREE(def->data.spice.agentmouse);
 VIR_FREE(def->data.spice.keymap);
 virDomainGraphicsAuthDefClear(&def->data.spice.auth);
 break;
@@ -5543,6 +5544,8 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
 VIR_FREE(autoport);
 }

+    def->data.spice.agentmouse = virXMLPropString(node, "agentmouse");
+
 def->data.spice.keymap = virXMLPropString(node, "keymap");

 if (virDomainGraphicsAuthDefParseXML(node, &def->data.spice.auth,
@@ -11364,6 +11367,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
 if (listenAddr)
 virBufferAsprintf(buf, " listen='%s'", listenAddr);

+    if (def->data.spice.agentmouse)
+    virBufferEscapeString(buf, " agentmouse='%s'",
+  def->data.spice.agentmouse);
+
 if (def->data.spice.keymap)
 virBufferEscapeString(buf, " keymap='%s'",
   def->data.spice.keymap);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 596be4d..e55995c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1072,6 +1072,7 @@ struct _virDomainGraphicsDef {
 struct {
 int port;
 int tlsPort;
+    char *agentmouse;
 char *keymap;
 virDomainGraphicsAuthDef auth;
 unsigned int autoport :1;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 01adf0d..531ecbe 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5391,6 +5391,10 @@ qemuBuildCommandLine(virConnectPtr conn,

 VIR_FREE(netAddr);

+    if (def->graphics[0]->data.spice.agentmouse)
+    virBufferAsprintf(&opt, ",agent-mouse=%s",
+  def->graphics[0]->data.spice.agentmouse);
+
 /* In the password case we set it via monitor command, to avoid
  * making it visible on CLI, so there's no use of password=XXX
  * in this bit of the code */
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args
b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args
index 681f7c2..746c116 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args
@@ -5,5 +5,5 @@ virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -hda \
 /dev/HostVG/QEMUGuest1 -chardev spicevmc,id=charchannel0,name=vdagent -device \
 virtserialport,bus=virtio-serial1.0,nr=3,chardev=charchannel0,id=channel0\
 ,name=com.redhat.spice.0 -usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\
-x509-dir=/etc/pki/libvirt-spice,tls-channel=main -device \
+agent-mouse=off,x509-dir=/etc/pki/libvirt-spice,tls-channel=main -device \
 virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml
b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml
index 6505b55..266a4ed 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml
@@ -23,7 +23,7 @@
 
   
 
-    
+    
   
 
 

--
Zhou Peng
Signed-off-by: Zhou Peng 

spice agent-mouse support

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f9654f1..79d5ac9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -852,6 +852,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
 break;
 
 case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+VIR_FREE(def->data.spice.agentmouse);
 VIR_FREE(def->data.spice.keymap);
 virDomainGraphicsAuthDefClear(&def->data.spice.auth);
 break;
@@ -5543,6 +5544,8 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
 VIR_FREE(autoport);
 }
 
+def->data.spice.agentmouse = virXMLPropString(node, "agentmouse");
+
 def->data.spice.keymap = virXMLPropString(node, "keymap");
 
 if (virDomainGraphicsAuthDefParseXML(node, &def->data.spice.auth,
@@ -11364,6 +11367,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
 if (listenAddr)
 virBufferAsprintf(buf, " listen='%s'", listenAddr);
 
+if (def->data.spice.agentmouse)
+virBufferEscapeString(buf, " agentmouse='%s'",
+  def->data.spice.agentmouse);
+
 if (def->data.spice.keymap)
 virBufferEscapeString(buf, " keymap='%s'",
   def->data.spice.keymap);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 596be4d..e55995c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1072,

[libvirt] Important, switch of server for libvirt.org

2012-02-29 Thread Daniel Veillard
  I learned yesterday that the lab hosting libvirt.org would
have power cut off for the week-end due to electical maintainance.
I took the opportunity to switch the server to a new box,
and just made the switch to the DNS, it may take up to 8 hours
to propagate though.
  I cut ssh access to the old server 194.199.20.115 and reactivated
all services on the new box 176.31.99.103 . For read only access
nobody should notice the switch, only commiters should notice that
and hopefully they are all sleeping right now :-)

  Grab me on IRC or by email if you notice somethings which doesn't
work right (the search is not setup yet and I need to reaactivate
various crons, but nearly everything should just work (TM))

Daniel

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

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


Re: [libvirt] [PATCH] Use the same MAC address that is defined in domain XML for attached-mac field.

2012-02-29 Thread Ansis Atteka
On Sat, Feb 18, 2012 at 7:07 PM, Laine Stump  wrote:

> On 02/17/2012 02:51 PM, Ansis Atteka wrote:
> >
> >
> > On Fri, Feb 17, 2012 at 10:55 AM, Laine Stump  > > wrote:
> >
> > On 02/16/2012 06:49 PM, Ansis Atteka wrote:
> > > Currently libvirt sets the attached-mac to altered MAC address
> > that has
> > > first byte set to FE. This patch will change that behavior by
> > using the
> > > original (unaltered) MAC address from the domain XML
> > configuration file.
> >
> > Maybe I didn't read thoroughly enough, but I don't see where it
> > changes
> > the behavior - in the cases where previously the first byte was set
> to
> > 0xFE, now you send discourage=true, and in the cases where it didn't,
> > now you send discourage=false.
> >
> > "discourage" means whether bridge should be discouraged to use the
> > newly added
> > TAP device's MAC address. Libvirt does that by setting the first MAC
> > address byte
> > high enough.
> >
> > And here is how this patch works:
> >
> >  1. Now in virNetDevTapCreateInBridgePort() function we always pass
> > exactly the same MAC address that was defined in XML.
> >  2. If "discourage" flag was set to true, then we create a copy of MAC
> > address and set its first byte to 0xFE
> >  3. virNetDevSetMAC() function would use the MAC address that was
> > product of #2
> >  4. while virNetDevOpenvswitchAddPort() function would use the
> > original MAC address that was passed in #1 (this code did not need
> > to be changed so most likely that was the reason why you did not
> > notice behavior changes)
> >
>
>
> Right. That's what I missed - all I saw was every occurrence of creating
> a temporary mac address with 0xFE in the first byte replaced with adding
> "discourage=true" to the args. I didn't notice that
> virNetDevOpenvswitchAddPort() takes the macaddr (while
> virNetDevBridgeAddPort() doesn't).
>
> But that means that the tap device has been created with an
> 0xFE-initiated MAC address, and then you attach to the bridge using the
> unmodified address. Is the issue that the mac address used during the
> attach needs to match the MAC address that will be in the traffic? Do
> connections to an openvswitch bridge have an implied MAC filter on them,
> such that only that MAC address gets through?
>
> (Also, the only time discourage is false is for libvirt's virtual
> network bridges. I'm wondering if they could also use the modified MAC
> address for the tap devices - if that was the case we could just always
> create the temporary MAC address in virNetDevTapCreateInBridgePort()
> (and always set the tap device's mac to that).)
>
We could get rid of the "discourage" argument if we would pass
virDomainNetDefPtr instead of virNetDevVPortProfilePtr structure to
virNetDevOpenvswitchAddPort() function. This approach would
also eliminate the need to pass MAC address at all to the
virNetDevOpenvswitchAddPort() function making both
APIs for Linux Bridge and OVS bridge more simpler and
similar (and this could eventually lead to abstracted bridge API).

But this would not solve the issue for Domain UUID which we also
would like to set from virNetDevOpenvswitchAddPort() function.
So what about adding pointer in virDomainNetDefPtr structure
to virDomainDefPtr structure?

If you guys are ok with what I am proposing here then I could send
my updated patches for review (one for attached-mac and another
for vm-uuid).


> Let's let this sit over the weekend and see if anyone else has a
> brilliant idea/insight.
>
> >
> > Is this a precursor to something else? Does openvswitch maybe not
> need
> > this extremely high MAC address?
> >
> >
> >
> >
> > > ---
> > >  src/network/bridge_driver.c |2 +-
> > >  src/qemu/qemu_command.c |5 +
> > >  src/uml/uml_conf.c  |5 +
> > >  src/util/virnetdevtap.c |   11 ++-
> > >  src/util/virnetdevtap.h |1 +
> > >  5 files changed, 14 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/src/network/bridge_driver.c
> > b/src/network/bridge_driver.c
> > > index 8575d3e..3e1e031 100644
> > > --- a/src/network/bridge_driver.c
> > > +++ b/src/network/bridge_driver.c
> > > @@ -1766,7 +1766,7 @@ networkStartNetworkVirtual(struct
> > network_driver *driver,
> > >  }
> > >  if (virNetDevTapCreateInBridgePort(network->def->bridge,
> > > &macTapIfName,
> > network->def->mac,
> > > -   0, false, NULL,
> > NULL) < 0) {
> > > +   false, 0, false,
> > NULL, NULL) < 0) {
> > >  VIR_FREE(macTapIfName);
> > >  goto err0;
> > >  }
> > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > > index 5a34504..671054c 100644
> > > --- 

[libvirt] [libvirt-glib 1/6] Set correct target node attribute for domain interface

2012-02-29 Thread Zeeshan Ali (Khattak)
From: "Zeeshan Ali (Khattak)" 

gvir_config_domain_interface_set_ifname() should be setting 'dev'
attribute under 'target', not 'device'.
---
 libvirt-gconfig/libvirt-gconfig-domain-interface.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c 
b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
index 85cc194..eab4313 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
@@ -57,7 +57,7 @@ void 
gvir_config_domain_interface_set_ifname(GVirConfigDomainInterface *interfac
 g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface));
 
 
gvir_config_object_replace_child_with_attribute(GVIR_CONFIG_OBJECT(interface),
-"target", "device", 
ifname);
+"target", "dev", ifname);
 }
 
 void gvir_config_domain_interface_set_link_state(GVirConfigDomainInterface 
*interface,
-- 
1.7.7.6

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


[libvirt] [libvirt-glib 5/6] Remove now redundant 'path' property

2012-02-29 Thread Zeeshan Ali (Khattak)
From: "Zeeshan Ali (Khattak)" 

Remove now redundant 'path' property from GVirDomainDevice subclasses.
These classes now have access to their configurations, from which they
can easily get the path (among other properties) internally.
---
 libvirt-gobject/libvirt-gobject-domain-disk.c  |   84 ---
 libvirt-gobject/libvirt-gobject-domain-interface.c |   81 +++
 2 files changed, 32 insertions(+), 133 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c 
b/libvirt-gobject/libvirt-gobject-domain-disk.c
index fb7672e..0a1493a 100644
--- a/libvirt-gobject/libvirt-gobject-domain-disk.c
+++ b/libvirt-gobject/libvirt-gobject-domain-disk.c
@@ -36,73 +36,25 @@
 
 struct _GVirDomainDiskPrivate
 {
-gchar *path;
+gboolean unused;
 };
 
 G_DEFINE_TYPE(GVirDomainDisk, gvir_domain_disk, GVIR_TYPE_DOMAIN_DEVICE);
 
-enum {
-PROP_0,
-PROP_PATH,
-};
-
 #define GVIR_DOMAIN_DISK_ERROR gvir_domain_disk_error_quark()
 
-
 static GQuark
 gvir_domain_disk_error_quark(void)
 {
 return g_quark_from_static_string("gvir-domain-disk");
 }
 
-static void gvir_domain_disk_get_property(GObject *object,
-  guint prop_id,
-  GValue *value,
-  GParamSpec *pspec)
-{
-GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
-GVirDomainDiskPrivate *priv = self->priv;
-
-switch (prop_id) {
-case PROP_PATH:
-g_value_set_string(value, priv->path);
-break;
-
-default:
-G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
-}
-}
-
-
-static void gvir_domain_disk_set_property(GObject *object,
-  guint prop_id,
-  const GValue *value,
-  GParamSpec *pspec)
-{
-GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
-GVirDomainDiskPrivate *priv = self->priv;
-
-switch (prop_id) {
-case PROP_PATH:
-g_free(priv->path);
-priv->path = g_value_dup_string(value);
-break;
-
-default:
-G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
-}
-}
-
-
 static void gvir_domain_disk_finalize(GObject *object)
 {
 GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
-GVirDomainDiskPrivate *priv = self->priv;
 
 g_debug("Finalize GVirDomainDisk=%p", self);
 
-g_free(priv->path);
-
 G_OBJECT_CLASS(gvir_domain_disk_parent_class)->finalize(object);
 }
 
@@ -111,19 +63,6 @@ static void gvir_domain_disk_class_init(GVirDomainDiskClass 
*klass)
 GObjectClass *object_class = G_OBJECT_CLASS (klass);
 
 object_class->finalize = gvir_domain_disk_finalize;
-object_class->get_property = gvir_domain_disk_get_property;
-object_class->set_property = gvir_domain_disk_set_property;
-
-g_object_class_install_property(object_class,
-PROP_PATH,
-g_param_spec_string("path",
-"Path",
-"The disk path",
-NULL,
-G_PARAM_READWRITE |
-G_PARAM_CONSTRUCT_ONLY 
|
-
G_PARAM_STATIC_STRINGS));
-
 g_type_class_add_private(klass, sizeof(GVirDomainDiskPrivate));
 }
 
@@ -151,6 +90,15 @@ gvir_domain_disk_stats_free(GVirDomainDiskStats *stats)
 G_DEFINE_BOXED_TYPE(GVirDomainDiskStats, gvir_domain_disk_stats,
 gvir_domain_disk_stats_copy, gvir_domain_disk_stats_free)
 
+static gchar *gvir_domain_disk_get_path(GVirDomainDisk *self)
+{
+GVirConfigDomainDevice *config;
+
+config = gvir_domain_device_get_config(GVIR_DOMAIN_DEVICE(self));
+
+return gvir_config_domain_disk_get_target_dev (GVIR_CONFIG_DOMAIN_DISK 
(config));
+}
+
 /**
  * gvir_domain_disk_get_stats:
  * @self: the domain disk
@@ -166,15 +114,15 @@ GVirDomainDiskStats 
*gvir_domain_disk_get_stats(GVirDomainDisk *self, GError **e
 {
 GVirDomainDiskStats *ret = NULL;
 virDomainBlockStatsStruct stats;
-GVirDomainDiskPrivate *priv;
 virDomainPtr handle;
+gchar *path;
 
 g_return_val_if_fail(GVIR_IS_DOMAIN_DISK(self), NULL);
 
-priv = self->priv;
 handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self));
+path = gvir_domain_disk_get_path (self);
 
-if (virDomainBlockStats(handle, priv->path, &stats, sizeof (stats)) < 0) {
+if (virDomainBlockStats(handle, path, &stats, sizeof (stats)) < 0) {
 gvir_set_error_literal(err, GVIR_DOMAIN_DISK_ERROR,
0,
"Unable to get domain disk stats");
@@ -190,6 +138,7 @@ GVirDomainDiskStats 
*gvir_domain_disk_get_stats(

[libvirt] [libvirt-glib 3/6] Getters for GVirConfigDomainInterface attributes

2012-02-29 Thread Zeeshan Ali (Khattak)
From: "Zeeshan Ali (Khattak)" 

---
 libvirt-gconfig/libvirt-gconfig-domain-interface.c |   35 
 libvirt-gconfig/libvirt-gconfig-domain-interface.h |4 ++
 libvirt-gconfig/libvirt-gconfig.sym|4 ++
 3 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c 
b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
index eab4313..5fe27a1 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
@@ -96,6 +96,41 @@ void 
gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface
 "model", "type", model);
 }
 
+char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface 
*interface)
+{
+g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL);
+
+return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface),
+"target", "dev");
+}
+
+GVirConfigDomainInterfaceLinkState 
gvir_config_domain_interface_get_link_state(GVirConfigDomainInterface 
*interface)
+{
+g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface),
+ GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT);
+
+return 
gvir_config_object_get_attribute_genum(GVIR_CONFIG_OBJECT(interface),
+  "link", "state",
+  
GVIR_CONFIG_TYPE_DOMAIN_INTERFACE_LINK_STATE,
+  
GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT);
+}
+
+char *gvir_config_domain_interface_get_mac(GVirConfigDomainInterface 
*interface)
+{
+g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL);
+
+return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface),
+"mac", "address");
+}
+
+char *gvir_config_domain_interface_get_model(GVirConfigDomainInterface 
*interface)
+{
+g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL);
+
+return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface),
+"model", "type");
+}
+
 G_GNUC_INTERNAL GVirConfigDomainDevice *
 gvir_config_domain_interface_new_from_tree(GVirConfigXmlDoc *doc,
xmlNodePtr tree)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.h 
b/libvirt-gconfig/libvirt-gconfig-domain-interface.h
index 6e802fb..567f95a 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-interface.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.h
@@ -72,6 +72,10 @@ void 
gvir_config_domain_interface_set_mac(GVirConfigDomainInterface *interface,
   const char *mac_address);
 void gvir_config_domain_interface_set_model(GVirConfigDomainInterface 
*interface,
 const char *model);
+char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface 
*interface);
+GVirConfigDomainInterfaceLinkState 
gvir_config_domain_interface_get_link_state(GVirConfigDomainInterface 
*interface);
+char *gvir_config_domain_interface_get_mac(GVirConfigDomainInterface 
*interface);
+char *gvir_config_domain_interface_get_model(GVirConfigDomainInterface 
*interface);
 
 G_END_DECLS
 
diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
b/libvirt-gconfig/libvirt-gconfig.sym
index 96ce58f..1329c17 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -142,9 +142,13 @@ LIBVIRT_GCONFIG_0.0.4 {
gvir_config_domain_interface_get_type;
gvir_config_domain_interface_link_state_get_type;
gvir_config_domain_interface_set_ifname;
+   gvir_config_domain_interface_get_ifname;
gvir_config_domain_interface_set_link_state;
+   gvir_config_domain_interface_get_link_state;
gvir_config_domain_interface_set_mac;
+   gvir_config_domain_interface_get_mac;
gvir_config_domain_interface_set_model;
+   gvir_config_domain_interface_get_model;
 
gvir_config_domain_interface_bridge_get_type;
gvir_config_domain_interface_bridge_new;
-- 
1.7.7.6

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


[libvirt] [libvirt-glib 4/6] Add 'config' property to GVirDomainDevice

2012-02-29 Thread Zeeshan Ali (Khattak)
From: "Zeeshan Ali (Khattak)" 

GVirDomainDevice should have an associated GVirConfigDomainDevice.
---
 libvirt-gobject/libvirt-gobject-domain-device.c |   33 +++
 libvirt-gobject/libvirt-gobject-domain-device.h |1 +
 libvirt-gobject/libvirt-gobject.sym |1 +
 3 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c 
b/libvirt-gobject/libvirt-gobject-domain-device.c
index 750c8d9..35d4855 100644
--- a/libvirt-gobject/libvirt-gobject-domain-device.c
+++ b/libvirt-gobject/libvirt-gobject-domain-device.c
@@ -37,6 +37,7 @@
 struct _GVirDomainDevicePrivate
 {
 GVirDomain *domain;
+GVirConfigDomainDevice *config;
 };
 
 G_DEFINE_ABSTRACT_TYPE(GVirDomainDevice, gvir_domain_device, G_TYPE_OBJECT);
@@ -44,6 +45,7 @@ G_DEFINE_ABSTRACT_TYPE(GVirDomainDevice, gvir_domain_device, 
G_TYPE_OBJECT);
 enum {
 PROP_0,
 PROP_DOMAIN,
+PROP_CONFIG,
 };
 
 static void gvir_domain_device_get_property(GObject *object,
@@ -59,6 +61,10 @@ static void gvir_domain_device_get_property(GObject *object,
 g_value_set_object(value, priv->domain);
 break;
 
+case PROP_CONFIG:
+g_value_set_object(value, priv->config);
+break;
+
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
 }
@@ -79,6 +85,11 @@ static void gvir_domain_device_set_property(GObject *object,
 priv->domain = g_value_dup_object(value);
 break;
 
+case PROP_CONFIG:
+g_clear_object(&priv->config);
+priv->config = g_value_dup_object(value);
+break;
+
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
 }
@@ -93,6 +104,7 @@ static void gvir_domain_device_finalize(GObject *object)
 g_debug("Finalize GVirDomainDevice=%p", self);
 
 g_clear_object(&priv->domain);
+g_clear_object(&priv->config);
 
 G_OBJECT_CLASS(gvir_domain_device_parent_class)->finalize(object);
 }
@@ -115,6 +127,16 @@ static void 
gvir_domain_device_class_init(GVirDomainDeviceClass *klass)
 G_PARAM_CONSTRUCT_ONLY 
|
 
G_PARAM_STATIC_STRINGS));
 
+g_object_class_install_property(object_class,
+PROP_CONFIG,
+g_param_spec_object("config",
+"Config",
+"The configuration",
+
GVIR_CONFIG_TYPE_DOMAIN_DEVICE,
+G_PARAM_READWRITE |
+G_PARAM_CONSTRUCT_ONLY 
|
+
G_PARAM_STATIC_STRINGS));
+
 g_type_class_add_private(klass, sizeof(GVirDomainDevicePrivate));
 }
 
@@ -145,3 +167,14 @@ GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice 
*device)
 {
 return g_object_ref (device->priv->domain);
 }
+
+/**
+ * gvir_domain_device_get_config:
+ * @device: the domain device
+ *
+ * Returns: (transfer full): the config
+ */
+GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice *device)
+{
+return g_object_ref (device->priv->config);
+}
diff --git a/libvirt-gobject/libvirt-gobject-domain-device.h 
b/libvirt-gobject/libvirt-gobject-domain-device.h
index 98acc2d..b308477 100644
--- a/libvirt-gobject/libvirt-gobject-domain-device.h
+++ b/libvirt-gobject/libvirt-gobject-domain-device.h
@@ -61,6 +61,7 @@ struct _GVirDomainDeviceClass
 
 GType gvir_domain_device_get_type(void);
 GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device);
+GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice 
*device);
 
 G_END_DECLS
 
diff --git a/libvirt-gobject/libvirt-gobject.sym 
b/libvirt-gobject/libvirt-gobject.sym
index 0097692..d6999dc 100644
--- a/libvirt-gobject/libvirt-gobject.sym
+++ b/libvirt-gobject/libvirt-gobject.sym
@@ -34,6 +34,7 @@ LIBVIRT_GOBJECT_0.0.4 {
 
gvir_domain_device_get_type;
gvir_domain_device_get_domain;
+   gvir_domain_device_get_config;
 
gvir_domain_disk_get_type;
gvir_domain_disk_stats_get_type;
-- 
1.7.7.6

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


[libvirt] [libvirt-glib 2/6] Add gvir_domain_device_get_domain()

2012-02-29 Thread Zeeshan Ali (Khattak)
From: "Zeeshan Ali (Khattak)" 

Getter for the associated domain of a domain device.
---
 libvirt-gobject/libvirt-gobject-domain-device.c |   11 +++
 libvirt-gobject/libvirt-gobject-domain-device.h |3 +++
 libvirt-gobject/libvirt-gobject.sym |1 +
 3 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c 
b/libvirt-gobject/libvirt-gobject-domain-device.c
index 528b513..750c8d9 100644
--- a/libvirt-gobject/libvirt-gobject-domain-device.c
+++ b/libvirt-gobject/libvirt-gobject-domain-device.c
@@ -134,3 +134,14 @@ virDomainPtr 
gvir_domain_device_get_domain_handle(GVirDomainDevice *self)
 
 return handle;
 }
+
+/**
+ * gvir_domain_device_get_domain:
+ * @device: the domain device
+ *
+ * Returns: (transfer full): the associated domain
+ */
+GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device)
+{
+return g_object_ref (device->priv->domain);
+}
diff --git a/libvirt-gobject/libvirt-gobject-domain-device.h 
b/libvirt-gobject/libvirt-gobject-domain-device.h
index 96c0433..98acc2d 100644
--- a/libvirt-gobject/libvirt-gobject-domain-device.h
+++ b/libvirt-gobject/libvirt-gobject-domain-device.h
@@ -27,6 +27,8 @@
 #ifndef __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__
 #define __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__
 
+#include 
+
 G_BEGIN_DECLS
 
 #define GVIR_TYPE_DOMAIN_DEVICE(gvir_domain_device_get_type ())
@@ -58,6 +60,7 @@ struct _GVirDomainDeviceClass
 
 
 GType gvir_domain_device_get_type(void);
+GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device);
 
 G_END_DECLS
 
diff --git a/libvirt-gobject/libvirt-gobject.sym 
b/libvirt-gobject/libvirt-gobject.sym
index 5081f41..0097692 100644
--- a/libvirt-gobject/libvirt-gobject.sym
+++ b/libvirt-gobject/libvirt-gobject.sym
@@ -33,6 +33,7 @@ LIBVIRT_GOBJECT_0.0.4 {
gvir_connection_get_node_info;
 
gvir_domain_device_get_type;
+   gvir_domain_device_get_domain;
 
gvir_domain_disk_get_type;
gvir_domain_disk_stats_get_type;
-- 
1.7.7.6

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


[libvirt] [libvirt-glib 6/6] Add gvir_domain_get_devices()

2012-02-29 Thread Zeeshan Ali (Khattak)
From: "Zeeshan Ali (Khattak)" 

Currently we only support existing DomainDevice implementations:
DomainDisk and DomainInterface.
---
 .../libvirt-gobject-domain-device-private.h|2 +
 libvirt-gobject/libvirt-gobject-domain-device.c|   22 ++
 libvirt-gobject/libvirt-gobject-domain.c   |   42 
 libvirt-gobject/libvirt-gobject-domain.h   |3 +
 libvirt-gobject/libvirt-gobject.sym|1 +
 5 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h 
b/libvirt-gobject/libvirt-gobject-domain-device-private.h
index 72c660e..a505ecd 100644
--- a/libvirt-gobject/libvirt-gobject-domain-device-private.h
+++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h
@@ -24,6 +24,8 @@
 
 G_BEGIN_DECLS
 
+G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirDomain *domain,
+ 
GVirConfigDomainDevice *config);
 virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self);
 
 G_END_DECLS
diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c 
b/libvirt-gobject/libvirt-gobject-domain-device.c
index 35d4855..f2195af 100644
--- a/libvirt-gobject/libvirt-gobject-domain-device.c
+++ b/libvirt-gobject/libvirt-gobject-domain-device.c
@@ -178,3 +178,25 @@ GVirConfigDomainDevice 
*gvir_domain_device_get_config(GVirDomainDevice *device)
 {
 return g_object_ref (device->priv->config);
 }
+
+G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirDomain *domain,
+ 
GVirConfigDomainDevice *config)
+{
+GType type;
+
+g_return_val_if_fail(GVIR_IS_DOMAIN(domain), NULL);
+g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), NULL);
+
+if (GVIR_CONFIG_IS_DOMAIN_DISK(config))
+type = GVIR_TYPE_DOMAIN_DISK;
+else if (GVIR_CONFIG_IS_DOMAIN_INTERFACE(config))
+type = GVIR_TYPE_DOMAIN_INTERFACE;
+else {
+g_debug("Unknown device type: %s", G_OBJECT_TYPE_NAME(config));
+return NULL;
+}
+
+return GVIR_DOMAIN_DEVICE(g_object_new(type,
+   "config", config,
+   "domain", domain, NULL));
+}
diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index 23ad882..ae86f0e 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -29,6 +29,7 @@
 #include "libvirt-glib/libvirt-glib.h"
 #include "libvirt-gobject/libvirt-gobject.h"
 #include "libvirt-gobject-compat.h"
+#include "libvirt-gobject/libvirt-gobject-domain-device-private.h"
 
 #define GVIR_DOMAIN_GET_PRIVATE(obj) \
 (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN, 
GVirDomainPrivate))
@@ -868,3 +869,44 @@ gboolean gvir_domain_get_saved(GVirDomain *dom)
 
 return virDomainHasManagedSaveImage(dom->priv->handle, 0) == 1;
 }
+
+/**
+ * gvir_domain_get_devices:
+ * @domain: the domain
+ * @err: place-holder for possible errors, or NULL
+ *
+ * Gets the list of devices attached to @domain
+ *
+ * Returns: (element-type LibvirtGObject.DomainDevice) (transfer full): a newly
+ * allocated #GList of #GVirDomainDevice.
+ */
+GList *gvir_domain_get_devices(GVirDomain *domain,
+   GError **err)
+{
+GVirConfigDomain *config;
+GList *config_devices;
+GList *node;
+GList *ret = NULL;
+
+g_return_val_if_fail(GVIR_IS_DOMAIN(domain), NULL);
+
+config = gvir_domain_get_config(domain, 0, err);
+if (config == NULL)
+return ret;
+
+config_devices = gvir_config_domain_get_devices(config);
+for (node = config_devices; node != NULL; node = node->next) {
+GVirConfigDomainDevice *device_config;
+GVirDomainDevice *device;
+
+device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data);
+device = gvir_domain_device_new(domain, device_config);
+if (device != NULL)
+ ret = g_list_prepend(ret, device);
+
+g_object_unref (device_config);
+}
+g_list_free (config_devices);
+
+return ret;
+}
diff --git a/libvirt-gobject/libvirt-gobject-domain.h 
b/libvirt-gobject/libvirt-gobject-domain.h
index 56500a8..8a4836e 100644
--- a/libvirt-gobject/libvirt-gobject-domain.h
+++ b/libvirt-gobject/libvirt-gobject-domain.h
@@ -183,6 +183,9 @@ gboolean gvir_domain_save_finish (GVirDomain *dom,
 gboolean gvir_domain_get_persistent(GVirDomain *dom);
 gboolean gvir_domain_get_saved(GVirDomain *dom);
 
+GList *gvir_domain_get_devices(GVirDomain *domain,
+   GError **err);
+
 G_END_DECLS
 
 #endif /* __LIBVIRT_GOBJECT_DOMAIN_H__ */
diff --git a/libvirt-gobject/libvirt-gobject.sym 
b/libvirt-gobject/libvirt-gobject.sym
index d6999dc..460280b 100644
--- a/libvirt-gobject/libvirt-gobject.sym
+++ b/libvirt-gobject/libvirt-gobject.sym
@@ -68,6 +

Re: [libvirt] [libvirt-glib 1/6] Getters for GVirConfigDomainInterface attributes

2012-02-29 Thread Zeeshan Ali (Khattak)
On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau  wrote:
> On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" 
>>
>> ---
>>  libvirt-gconfig/libvirt-gconfig-domain-interface.c |   35 
>> 
>>  libvirt-gconfig/libvirt-gconfig-domain-interface.h |    4 ++
>>  libvirt-gconfig/libvirt-gconfig.sym                |    4 ++
>>  3 files changed, 43 insertions(+), 0 deletions(-)
>>
>> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c 
>> b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> index 85cc194..61d35bd 100644
>> --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> @@ -96,6 +96,41 @@ void 
>> gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface
>>                                                      "model", "type", model);
>>  }
>>
>> +const char 
>> *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface 
>> *interface)
>
> Unless I'm missing something, this should not be const (caller needs to
> free the returned string).
>
>> +{
>> +    g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL);
>> +
>> +    return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface),
>> +                                            "target", "device");
>
> This is "dev", not "device"

Turns out that i copy&pasted the "device" from the corresponding
setter. Wonder if this explains why I see a flat network graph in
boxes for every domain..

-- 
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 3/3] virsh: expose partial pull

2012-02-29 Thread Eric Blake
On 02/29/2012 03:08 AM, Laine Stump wrote:
> On 02/18/2012 12:44 PM, Eric Blake wrote:
>> Now virsh can call virDomainBlockRebase.
> 
> ACK.

Series 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] build: update to latest gnulib

2012-02-29 Thread Eric Blake
On 02/29/2012 02:50 AM, Laine Stump wrote:
> On 02/24/2012 07:39 PM, Eric Blake wrote:
>> 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.
> 
> I just noticed this hasn't been ACKed/pushed yet.
> 
> Sounds like a good idea to me. ACK.

It turns out I found a build-breaker after all - libvirt failed to
compile during 'make check' on cygwin 1.7.11 unless I further update
.gnulib for a few more fixes.  Now pushed, with .gnulib moved up to:

* .gnulib e9e8aba...b856fad (180):
  > termios: fix pid_t always, not just for tcgetsid
  > update from texinfo
  > Tests for module 'hypotl'.
  > New module 'hypotl'.
  > hypotf: Fix typo in comment.
  > tcgetsid: fix cygwin header bug
  > docs: update cygwin progress
  > Tests for module 'hypotf'.
  > New module 'hypotf'.
  > hypot: Prepare for hypotf module.
  > hypot tests: More tests.
  > math code: Add comments.
  > math: Ensure HUGE_VAL, HUGE_VALF, HUGE_VALL are defined.
  > doc: Move ISO C11 feature notes into POSIX chapters.
  > stdnoreturn: port to MSVC better
  > doc: Mention new glibc headers and functions.
  > Avoid compilation errors with MSVC option -fp:strict.
  > Tests for module 'sqrtl-ieee'.
  > New module 'sqrtl-ieee'.
  > Tests for module 'sqrt-ieee'.
  > New module 'sqrt-ieee'.
  > Tests for module 'sqrtf-ieee'.
  > New module 'sqrtf-ieee'.
  > remainderl-ieee: Work around test failure on OSF/1.
  > remainderf-ieee: Work around test failure on OSF/1.
  > remainder-ieee: Work around test failure on OSF/1.
  > Tests for module 'remainderl-ieee'.
  > New module 'remainderl-ieee'.
  > Tests for module 'remainder-ieee'.
  > New module 'remainder-ieee'.
  > Tests for module 'remainderf-ieee'.
  > New module 'remainderf-ieee'.
  > modff, modfl: Fix configure syntax error.
  > fmodl-ieee: Work around test failures on OSF/1, MSVC 9.
  > fmodf-ieee: Work around test failure on OSF/1.
  > fmodf-ieee: Work around test failure on MSVC 9.
  > fmod-ieee: Work around test failures on OSF/1, mingw.
  > fmodl-ieee: Fix test failures.
  > Tests for module 'fmodl-ieee'.
  > New module 'fmodl-ieee'.
  > Tests for module 'fmod-ieee'.
  > New module 'fmod-ieee'.
  > Tests for module 'fmodf-ieee'.
  > New module 'fmodf-ieee'.
  > Tests for module 'rintl-ieee'.
  > New module 'rintl-ieee'.
  > Tests for module 'rint-ieee'.
  > New module 'rint-ieee'.
  > Tests for module 'rintf-ieee'.
  > New module 'rintf-ieee'.
  > regex: re_search etc. should return -2 when memory exhausted
  > modfl-ieee: Work around test failures on IRIX, OSF/1, mingw.
  > modfl-ieee: Fix dependencies.
  > modfl-ieee: Fix test failures.
  > modff-ieee: Work around test failures on *BSD, IRIX, OSF/1, etc.
  > modf-ieee: Work around test failures on *BSD, IRIX, OSF/1, Cygwin.
  > Tests for module 'modfl-ieee'.
  > New module 'modfl-ieee'.
  > Tests for module 'modf-ieee'.
  > New module 'modf-ieee'.
  > Tests for module 'modff-ieee'.
  > New module 'modff-ieee'.
  > Tests for module 'fabsl-ieee'.
  > New module 'fabsl-ieee'.
  > Tests for module 'fabs-ieee'.
  > New module 'fabs-ieee'.
  > Tests for module 'fabsf-ieee'.
  > New module 'fabsf-ieee'.
  > fma*-ieee tests: Remove unneeded dependency.
  > Tests for module 'fmal-ieee'.
  > New module 'fmal-ieee'.
  > Tests for module 'fma-ieee'.
  > New module 'fma-ieee'.
  > Tests for module 'fmaf-ieee'.
  > New module 'fmaf-ieee'.
  > Tests for module 'ldexpl-ieee'.
  > New module 'ldexpl-ieee'.
  > Tests for module 'ldexp-ieee'.
  > New module 'ldexp-ieee'.
  > Tests for module 'ldexpf-ieee'.
  > New module 'ldexpf-ieee'.
  > Refactor frexp*-ieee tests.
  > More tests for modules frexpf-ieee, frexp-ieee, frexpl-ieee.
  > Tests for module 'frexpl-ieee'.
  > New module 'frexpl-ieee'.
  > Tests for module 'frexp-ieee'.
  > New module 'frexp-ieee'.
  > Tests for module 'frexpf-ieee'.
  > New module 'frexpf-ieee'.
  > roundl-ieee tests: More tests.
  > round-ieee tests: More tests.
  > roundf-ieee tests: More tests.
  > truncl-ieee tests: More tests.
  > trunc-ieee tests: More tests.
  > truncf-ieee tests: More tests.
  > ceill-ieee tests: More tests.
  > ceil-ieee tests: More tests.
  > ceilf-ieee tests: More tests.
  > floorl-ieee tests: More tests.
  > floor-ieee tests: More tests.
  > floorf-ieee tests: More tests.
  > fpieee: More comments.
  > Tests for module 'log10l'.
  > New module 'log10l'.
  > fmodl, remainder*: Avoid wrong results due to rounding errors.
  > Fix typo in recent ChangeLog entry.
  > Tests for module 'remainderl'.
  > New module 'remainderl'.
  > Tests for module 'remainderf'.
  > New module 'remainderf'.
  > remainder: Support for MSVC.
  > Tests for module 'fmodl'.
  > New module 'fmodl'.
  > Tests for module 'modfl'.
  > New module 'modfl'.
  > Tests for module 'fabsl'.
  > Tests for module 'fabsl'.
  > New module 'fabsl'.
  > fabs tests: More tests.
  > fabsf tests:

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

2012-02-29 Thread Laine Stump
On 02/24/2012 09:29 AM, D. Herrendoerfer wrote:
> 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.

This seems very problematic to me - it is assumed that libvirt can be
restarted at any time with no ill effects, and restarting libvirt is
often suggested as a solution to clearing up problems (e.g. if some
other program stomps on libvirt's iptables rules).

What can be done to eliminate this problem? Perhaps there needs to be a
"re-associate" message - it could be sent each time libvirt restarts for
each association it's currently tracking.

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


Re: [libvirt] [PATCH] Fix typo in domain XML documentation

2012-02-29 Thread Eric Blake
On 02/29/2012 05:47 AM, Christophe Fergeau wrote:
> s/Modyfing/Modifying
> ---
>  docs/formatdomain.html.in |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

ACK.  You can push patches like this under the trivial rule (see
HACKING); it's a good idea to still send the mail, but just mention that
you are pushing after the --- line and it cuts down on others having to
reply with an 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

Re: [libvirt] [libvirt-glib 6/6] Add gvir_domain_get_devices()

2012-02-29 Thread Zeeshan Ali (Khattak)
On Wed, Feb 29, 2012 at 4:24 PM, Christophe Fergeau  wrote:
> On Wed, Feb 29, 2012 at 04:01:40PM +0200, Zeeshan Ali (Khattak) wrote:
>> On Wed, Feb 29, 2012 at 3:30 PM, Christophe Fergeau  
>> wrote:
>> > On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
>> >> From: "Zeeshan Ali (Khattak)" 
>> >>
>> >> Currently we only support existing DomainDevice implementations:
>> >> DomainDisk and DomainInterface.
>> >> ---
>> >>  .../libvirt-gobject-domain-device-private.h        |    2 +
>> >>  libvirt-gobject/libvirt-gobject-domain-device.c    |   21 ++
>> >>  libvirt-gobject/libvirt-gobject-domain.c           |   42 
>> >> 
>> >>  libvirt-gobject/libvirt-gobject-domain.h           |    3 +
>> >>  libvirt-gobject/libvirt-gobject.sym                |    1 +
>> >>  5 files changed, 69 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h 
>> >> b/libvirt-gobject/libvirt-gobject-domain-device-private.h
>> >> index 72c660e..292a2ac 100644
>> >> --- a/libvirt-gobject/libvirt-gobject-domain-device-private.h
>> >> +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h
>> >> @@ -24,6 +24,8 @@
>> >>
>> >>  G_BEGIN_DECLS
>> >>
>> >> +G_GNUC_INTERNAL GVirDomainDevice 
>> >> *gvir_domain_device_new(GVirConfigDomainDevice *config,
>> >> +                                                         GVirDomain 
>> >> *domain);
>> >>  virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice 
>> >> *self);
>> >>
>> >>  G_END_DECLS
>> >> diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c 
>> >> b/libvirt-gobject/libvirt-gobject-domain-device.c
>> >> index 85879d2..0ec5dad 100644
>> >> --- a/libvirt-gobject/libvirt-gobject-domain-device.c
>> >> +++ b/libvirt-gobject/libvirt-gobject-domain-device.c
>> >> @@ -176,3 +176,24 @@ GVirDomain 
>> >> *gvir_domain_device_get_domain(GVirDomainDevice *device) {
>> >>  GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice 
>> >> *device) {
>> >>      return g_object_ref (device->priv->config);
>> >>  }
>> >> +
>> >> +G_GNUC_INTERNAL GVirDomainDevice 
>> >> *gvir_domain_device_new(GVirConfigDomainDevice *config,
>> >> +                                                         GVirDomain 
>> >> *domain)
>> >
>> > gvir_domain_new_device(GVirDomain *domain, GVirConfigDomainDevice *config)
>> > maybe ?
>> > Having the argument that is always non-NULL first feels better to me.
>>
>> Who said config is nullable? :)
>
> I assumed this because of the lack of 
> g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), ...)

Ah, should fix that.

> It still kind of sounds better to me to say "let's create a device
> associated with this GVirDomain", ie to have this method operating on a
> GVirDomain.

Hmm.. OK, i'll change the order.

>> >> +
>> >> +        device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data);
>> >> +        device = gvir_domain_device_new(device_config, domain);
>> >> +        if (device != NULL)
>> >> +             ret = g_list_append(ret, device);
>> >
>> > slight preference for g_list_prepend (much more efficient when the list is
>> > big).
>>
>> Good point but wouldn't you expect devices to be sorted in the order
>> they are sorted in the XML?
>
> You can always do a g_list_reverse after the fact, and I'd rather we avoid
> to make such guarantees anyway. If gvir_config_domain_get_devices used
> g_list_prepend (which I just noticed it's not doing), you'd need to prepend
> to return things in the order of the XML :)
> Not that important here, it's just a better practice to use g_list_prepend.

OK. Will change this too then..

-- 
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] [libvirt-glib 1/6] Getters for GVirConfigDomainInterface attributes

2012-02-29 Thread Christophe Fergeau
On Wed, Feb 29, 2012 at 05:26:58PM +0200, Zeeshan Ali (Khattak) wrote:
> There wont' be any need for freeing if
> gvir_config_xml_get_child_element_content() and similar functions
> returned chid_node->content rather than  xmlNodeGetContent
> (child_node). That way you never need to free any string. The original
> gupnp-av code that you based this xml utils code on, does exactly
> that. I wonder why you changed it?

I didn't copy and paste the code nor look at it that closely.
Since libxml2 has public getters which seem to be doing complicated things,
it felt safer to use them :)

Christophe


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

Re: [libvirt] [libvirt-glib 1/6] Getters for GVirConfigDomainInterface attributes

2012-02-29 Thread Zeeshan Ali (Khattak)
On Wed, Feb 29, 2012 at 4:22 PM, Christophe Fergeau  wrote:
> On Wed, Feb 29, 2012 at 04:10:29PM +0200, Zeeshan Ali (Khattak) wrote:
>> Ah, I just realized why that is so:
>>
>> static char *libxml_str_to_glib(xmlChar *str)
>> {
>>     char *g_str;
>>
>>     if (str == NULL)
>>         return NULL;
>>     g_str = g_strdup((char *)str);
>>     xmlFree(str);
>>
>>     return g_str;
>> }
>>
>> This function is not needed as all you needed was to cast the 'xmlChar
>> *' to 'const gchar *' and return const from all users of this
>> function. Since we still are not API/ABI stable, I propose we change
>> this all over as there is no need to force apps to free strings all
>> the time and waste processor/memory on all these string
>> allocation/de-allocation.
>
> You'll still need to free the input "str", and you have no guarantee that
> xmlFree and g_free will call the same function to free memory in the end,

There wont' be any need for freeing if
gvir_config_xml_get_child_element_content() and similar functions
returned chid_node->content rather than  xmlNodeGetContent
(child_node). That way you never need to free any string. The original
gupnp-av code that you based this xml utils code on, does exactly
that. I wonder why you changed it?

> If you really insist on returning const from your getters, you'll need to
> cache their value in GVirDomainDevicePrivate

Not if its const returned from all functions.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

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

[libvirt] [PATCH] cpu: Add new flag supported by qemu to the cpu definition

2012-02-29 Thread Peter Krempa
Some new cpu features were added to qemu. This patch adds some of them
to our CPU map.
---
to ease review, here's an excerpt from qemu.git/target-i386/cpuid.c to ease the 
review:

static const char *ext3_feature_name[] = {
"lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD 
ExtApicSpace */,
"cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
"3dnowprefetch", "osvw", "ibs", "xop",
"skinit", "wdt", NULL, NULL,
"fma4", NULL, "cvt16", "nodeid_msr",
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
};

(each line corresponds to one order of magnitude in the hex representation)

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

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 3e6810f..4ce3131 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -236,12 +236,27 @@
  
   
 
+
+  
+
+
+  
+
  
   
 
 
   
 
+
+  
+
+
+  
+
+
+  
+

 
 
-- 
1.7.3.4

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


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

2012-02-29 Thread Jiri Denemark
On Fri, Feb 24, 2012 at 10:44:19 +0100, Martin Kletzander wrote:
> 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)

ACK and pushed.

Jirka

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


Re: [libvirt] [libvirt-glib 1/6] Getters for GVirConfigDomainInterface attributes

2012-02-29 Thread Christophe Fergeau
On Wed, Feb 29, 2012 at 04:10:29PM +0200, Zeeshan Ali (Khattak) wrote:
> Ah, I just realized why that is so:
> 
> static char *libxml_str_to_glib(xmlChar *str)
> {
> char *g_str;
> 
> if (str == NULL)
> return NULL;
> g_str = g_strdup((char *)str);
> xmlFree(str);
> 
> return g_str;
> }
> 
> This function is not needed as all you needed was to cast the 'xmlChar
> *' to 'const gchar *' and return const from all users of this
> function. Since we still are not API/ABI stable, I propose we change
> this all over as there is no need to force apps to free strings all
> the time and waste processor/memory on all these string
> allocation/de-allocation.

You'll still need to free the input "str", and you have no guarantee that
xmlFree and g_free will call the same function to free memory in the end,
especially in a library. This function is sucky, but there is no clean way
around it as far as I know.
If you really insist on returning const from your getters, you'll need to
cache their value in GVirDomainDevicePrivate

Christophe


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

Re: [libvirt] [libvirt-glib 6/6] Add gvir_domain_get_devices()

2012-02-29 Thread Christophe Fergeau
On Wed, Feb 29, 2012 at 04:01:40PM +0200, Zeeshan Ali (Khattak) wrote:
> On Wed, Feb 29, 2012 at 3:30 PM, Christophe Fergeau  
> wrote:
> > On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
> >> From: "Zeeshan Ali (Khattak)" 
> >>
> >> Currently we only support existing DomainDevice implementations:
> >> DomainDisk and DomainInterface.
> >> ---
> >>  .../libvirt-gobject-domain-device-private.h        |    2 +
> >>  libvirt-gobject/libvirt-gobject-domain-device.c    |   21 ++
> >>  libvirt-gobject/libvirt-gobject-domain.c           |   42 
> >> 
> >>  libvirt-gobject/libvirt-gobject-domain.h           |    3 +
> >>  libvirt-gobject/libvirt-gobject.sym                |    1 +
> >>  5 files changed, 69 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h 
> >> b/libvirt-gobject/libvirt-gobject-domain-device-private.h
> >> index 72c660e..292a2ac 100644
> >> --- a/libvirt-gobject/libvirt-gobject-domain-device-private.h
> >> +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h
> >> @@ -24,6 +24,8 @@
> >>
> >>  G_BEGIN_DECLS
> >>
> >> +G_GNUC_INTERNAL GVirDomainDevice 
> >> *gvir_domain_device_new(GVirConfigDomainDevice *config,
> >> +                                                         GVirDomain 
> >> *domain);
> >>  virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self);
> >>
> >>  G_END_DECLS
> >> diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c 
> >> b/libvirt-gobject/libvirt-gobject-domain-device.c
> >> index 85879d2..0ec5dad 100644
> >> --- a/libvirt-gobject/libvirt-gobject-domain-device.c
> >> +++ b/libvirt-gobject/libvirt-gobject-domain-device.c
> >> @@ -176,3 +176,24 @@ GVirDomain 
> >> *gvir_domain_device_get_domain(GVirDomainDevice *device) {
> >>  GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice 
> >> *device) {
> >>      return g_object_ref (device->priv->config);
> >>  }
> >> +
> >> +G_GNUC_INTERNAL GVirDomainDevice 
> >> *gvir_domain_device_new(GVirConfigDomainDevice *config,
> >> +                                                         GVirDomain 
> >> *domain)
> >
> > gvir_domain_new_device(GVirDomain *domain, GVirConfigDomainDevice *config)
> > maybe ?
> > Having the argument that is always non-NULL first feels better to me.
> 
> Who said config is nullable? :)

I assumed this because of the lack of 
g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), ...)
It still kind of sounds better to me to say "let's create a device
associated with this GVirDomain", ie to have this method operating on a
GVirDomain.

> 
> >> +{
> >> +    GType type;
> >> +
> >> +    g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), NULL);
> >> +
> >> +    if (GVIR_CONFIG_IS_DOMAIN_DISK(config))
> >> +        type = GVIR_TYPE_DOMAIN_DISK;
> >> +    else if (GVIR_CONFIG_IS_DOMAIN_INTERFACE(config))
> >> +        type = GVIR_TYPE_DOMAIN_INTERFACE;
> >> +    else {
> >> +        g_debug("Unknown device type: %s", G_OBJECT_TYPE_NAME(config));
> >> +        return NULL;
> >> +    }
> >
> > I'd have a slight preference for something like
> >
> > switch (G_OBJECT_TYPE(config)) {
> > case GVIR_CONFIG_TYPE_DOMAIN_DISK:
> >    type = GVIR_TYPE_DOMAIN_DISK;
> >    break;
> > ...
> >
> > but I'm fine with either version.
> 
> I would have prefered it too but if any of these classes are
> subclassed tomorrow and we forget to update this code, we'll start
> loosing config instances we can actually handle.

Ah good point.

> 
> >> +
> >> +        device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data);
> >> +        device = gvir_domain_device_new(device_config, domain);
> >> +        if (device != NULL)
> >> +             ret = g_list_append(ret, device);
> >
> > slight preference for g_list_prepend (much more efficient when the list is
> > big).
> 
> Good point but wouldn't you expect devices to be sorted in the order
> they are sorted in the XML?

You can always do a g_list_reverse after the fact, and I'd rather we avoid
to make such guarantees anyway. If gvir_config_domain_get_devices used
g_list_prepend (which I just noticed it's not doing), you'd need to prepend
to return things in the order of the XML :)
Not that important here, it's just a better practice to use g_list_prepend.

> 
> >> +
> >> +        g_object_unref (device_config);
> >> +    }
> >> +    g_list_free (config_devices);
> >
> > The individual elements need to be unref'ed too as well as config.
> 
> Thats what g_object_unref is doing just above? I think I should name

True, I didn't pay enough attention.

> this 'device_configs' to be consistent with 'device_config' and
> therefore clear..

I could pretend that's what confused me... :) This would be better indeed
now that you say it.

Christophe


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

Re: [libvirt] [libvirt-glib 1/6] Getters for GVirConfigDomainInterface attributes

2012-02-29 Thread Zeeshan Ali (Khattak)
On Wed, Feb 29, 2012 at 3:49 PM, Christophe Fergeau  wrote:
> On Wed, Feb 29, 2012 at 03:44:20PM +0200, Zeeshan Ali (Khattak) wrote:
>> On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau  
>> wrote:
>> > On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
>> >> From: "Zeeshan Ali (Khattak)" 
>> >>
>> >> ---
>> >>  libvirt-gconfig/libvirt-gconfig-domain-interface.c |   35 
>> >> 
>> >>  libvirt-gconfig/libvirt-gconfig-domain-interface.h |    4 ++
>> >>  libvirt-gconfig/libvirt-gconfig.sym                |    4 ++
>> >>  3 files changed, 43 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c 
>> >> b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> >> index 85cc194..61d35bd 100644
>> >> --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> >> +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> >> @@ -96,6 +96,41 @@ void 
>> >> gvir_config_domain_interface_set_model(GVirConfigDomainInterface 
>> >> *interface
>> >>                                                      "model", "type", 
>> >> model);
>> >>  }
>> >>
>> >> +const char 
>> >> *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface 
>> >> *interface)
>> >
>> > Unless I'm missing something, this should not be const (caller needs to
>> > free the returned string).
>>
>> String getters usually do return const in the gobject world[1] as
>> opposed to object getters as one require allocation/de-allocation and
>> the other only requires incrementing/decrementing a counter. Also is
>> the fact that strings are readily usable so you can just make a call
>> to the getter and thats it but objects are not usually readily usable
>> in the sense that you have to pass it to to some other function to do
>> something with it. Note that I'm just making educated guesses here as
>> to why 'const' for strings makes more sense as to why this practice is
>> followed.
>
> Strings are const in the gobject world when they are owned by the object
> and not by the caller of the getter (ie the caller doesn't need to free
> them). They are non-const when the caller has to free them. In this case,
> the strings need to be freed by the caller, thus "const" should not be
> used.

Ah, I just realized why that is so:

static char *libxml_str_to_glib(xmlChar *str)
{
char *g_str;

if (str == NULL)
return NULL;
g_str = g_strdup((char *)str);
xmlFree(str);

return g_str;
}

This function is not needed as all you needed was to cast the 'xmlChar
*' to 'const gchar *' and return const from all users of this
function. Since we still are not API/ABI stable, I propose we change
this all over as there is no need to force apps to free strings all
the time and waste processor/memory on all these string
allocation/de-allocation.

-- 
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] [libvirt-glib 6/6] Add gvir_domain_get_devices()

2012-02-29 Thread Zeeshan Ali (Khattak)
On Wed, Feb 29, 2012 at 3:30 PM, Christophe Fergeau  wrote:
> On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" 
>>
>> Currently we only support existing DomainDevice implementations:
>> DomainDisk and DomainInterface.
>> ---
>>  .../libvirt-gobject-domain-device-private.h        |    2 +
>>  libvirt-gobject/libvirt-gobject-domain-device.c    |   21 ++
>>  libvirt-gobject/libvirt-gobject-domain.c           |   42 
>> 
>>  libvirt-gobject/libvirt-gobject-domain.h           |    3 +
>>  libvirt-gobject/libvirt-gobject.sym                |    1 +
>>  5 files changed, 69 insertions(+), 0 deletions(-)
>>
>> diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h 
>> b/libvirt-gobject/libvirt-gobject-domain-device-private.h
>> index 72c660e..292a2ac 100644
>> --- a/libvirt-gobject/libvirt-gobject-domain-device-private.h
>> +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h
>> @@ -24,6 +24,8 @@
>>
>>  G_BEGIN_DECLS
>>
>> +G_GNUC_INTERNAL GVirDomainDevice 
>> *gvir_domain_device_new(GVirConfigDomainDevice *config,
>> +                                                         GVirDomain 
>> *domain);
>>  virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self);
>>
>>  G_END_DECLS
>> diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c 
>> b/libvirt-gobject/libvirt-gobject-domain-device.c
>> index 85879d2..0ec5dad 100644
>> --- a/libvirt-gobject/libvirt-gobject-domain-device.c
>> +++ b/libvirt-gobject/libvirt-gobject-domain-device.c
>> @@ -176,3 +176,24 @@ GVirDomain 
>> *gvir_domain_device_get_domain(GVirDomainDevice *device) {
>>  GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice 
>> *device) {
>>      return g_object_ref (device->priv->config);
>>  }
>> +
>> +G_GNUC_INTERNAL GVirDomainDevice 
>> *gvir_domain_device_new(GVirConfigDomainDevice *config,
>> +                                                         GVirDomain *domain)
>
> gvir_domain_new_device(GVirDomain *domain, GVirConfigDomainDevice *config)
> maybe ?
> Having the argument that is always non-NULL first feels better to me.

Who said config is nullable? :)

>> +{
>> +    GType type;
>> +
>> +    g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), NULL);
>> +
>> +    if (GVIR_CONFIG_IS_DOMAIN_DISK(config))
>> +        type = GVIR_TYPE_DOMAIN_DISK;
>> +    else if (GVIR_CONFIG_IS_DOMAIN_INTERFACE(config))
>> +        type = GVIR_TYPE_DOMAIN_INTERFACE;
>> +    else {
>> +        g_debug("Unknown device type: %s", G_OBJECT_TYPE_NAME(config));
>> +        return NULL;
>> +    }
>
> I'd have a slight preference for something like
>
> switch (G_OBJECT_TYPE(config)) {
> case GVIR_CONFIG_TYPE_DOMAIN_DISK:
>    type = GVIR_TYPE_DOMAIN_DISK;
>    break;
> ...
>
> but I'm fine with either version.

I would have prefered it too but if any of these classes are
subclassed tomorrow and we forget to update this code, we'll start
loosing config instances we can actually handle.

>> +
>> +        device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data);
>> +        device = gvir_domain_device_new(device_config, domain);
>> +        if (device != NULL)
>> +             ret = g_list_append(ret, device);
>
> slight preference for g_list_prepend (much more efficient when the list is
> big).

Good point but wouldn't you expect devices to be sorted in the order
they are sorted in the XML?

>> +
>> +        g_object_unref (device_config);
>> +    }
>> +    g_list_free (config_devices);
>
> The individual elements need to be unref'ed too as well as config.

Thats what g_object_unref is doing just above? I think I should name
this 'device_configs' to be consistent with 'device_config' and
therefore clear..

>> diff --git a/libvirt-gobject/libvirt-gobject.sym 
>> b/libvirt-gobject/libvirt-gobject.sym
>> index d6999dc..b7b95cb 100644
>> --- a/libvirt-gobject/libvirt-gobject.sym
>> +++ b/libvirt-gobject/libvirt-gobject.sym
>> @@ -72,6 +72,7 @@ LIBVIRT_GOBJECT_0.0.4 {
>>       gvir_domain_get_persistent;
>>       gvir_domain_get_saved;
>>       gvir_domain_screenshot;
>> +     gvir_domain_get_devices;
>
> Try to get the list sorted/consistent with the other entries.

K.

-- 
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 v2] Cleanup of the quick dirty fix from last week

2012-02-29 Thread Jiri Denemark
On Fri, Feb 10, 2012 at 11:13:54 -0700, Eric Blake wrote:
> On 02/10/2012 08:22 AM, Martin Kletzander wrote:
> > Just a cleanup of commit 32f881c6c42f94da70a3782fe20a058fe3dc39cc.
> > ---
> >  src/lxc/lxc_container.c |   17 +++--
> >  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> ACK.

I changed the subject since referring to a fix from last week is not really
valid now and pushed this forgotten patch.

Jirka

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


Re: [libvirt] [PATCH RFC]: Support numad

2012-02-29 Thread Bill Burns

On 02/28/2012 11:34 PM, Osier Yang wrote:

On 02/29/2012 12:40 AM, Daniel P. Berrange wrote:

On Tue, Feb 28, 2012 at 11:33:03AM -0500, Dave Allan wrote:

On Tue, Feb 28, 2012 at 10:10:50PM +0800, Osier Yang wrote:

numad is an user-level daemon that monitors NUMA topology and
processes resource consumption to facilitate good NUMA resource
alignment of applications/virtual machines to improve performance
and minimize cost of remote memory latencies. It provides a
pre-placement advisory interface, so significant processes can
be pre-bound to nodes with sufficient available resources.

More details: http://fedoraproject.org/wiki/Features/numad

"numad -w ncpus:memory_amount" is the advisory interface numad
provides currently.

This patch add the support by introducing new XML like:





Isn't the usual case going to be the vcpus and memory in the guest?
IMO we should default to passing those numbers to numad if
required_cpus and required_memory are not provided explicitly.


Indeed, why you would want to specify anything different ? At
first glance my reaction was just skip the XML and call numad
internally automatically with the guest configured allocation



Here the "required_cpus" stands for the physical CPUs number,
which will be used numad to choose the proper nodeset. So from
sementics point of view, it's different with 4,
I can imagine two problems if we reuse the vCPUs number for
numad's use:

1) Suppose there are 16 pCPUs, but the specified vCPUs number
is "64". I'm not sure if numad will work properly in this case,
but isn't it a bad use case? :-)

2) Suppose there are 128 pCPUs, but the specified vCPUs number
is "2". numad will work definitely, but is that the result the
user wants to see? no good to performace.

The basic thought is we provide the interface, and how to configure
the provided XML for good performace is on the end-user then. If
we mixed-use the two different sementics, and do things secrectly
in the codes, then I could imagine there will be performance problems.

The "required_memory" could be omitted though, we can reuse
"524288", but I'm not sure if it's good to
always pass a "memory amount" to numad command line, it may be
not good in some case. @Bill(s), correct me if I'm not right. :-)

Perhaps we could have a bool attribute then, such as:





Please keep Bill Gray on this thread. He is the author of numad and
is the best person to answer the above questions.

 Bill


Regards,
Osier


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


Re: [libvirt] [PATCH 1/1] complete netlink event integration

2012-02-29 Thread Laine Stump
On 02/29/2012 08:43 AM, D. Herrendoerfer wrote:
>
> On Feb 29, 2012, at 9:51 AM, Laine Stump wrote:
>
>> Since I found a couple other problems, but have made you suffer through
>> enough back and forth already, I've made some final suggested changes
>> myself, and am sending a diff patch as a response to this message
>> (differences from your latest version to mine), as well as doing a
>> repost of the original two patches with your latest changes and mine
>> squashed in.
>>
>> Please 1) take a look at the changes in my diff patches , then 2) test
>> the updated versions of the full patches (I'm unable to test beyond
>> compiling), and send your ACK if they're okay. Once I have your ACK
>> back, I'll push (I promise, I won't find any new issues *this* time :-)
>>
>
> ACK.
>
> The code tests out fine. One nit I found in the logs was that the removal
> was indicated by the wrong method:
>
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index fd6f751..614f8da 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -509,7 +509,7 @@ virNetlinkEventRemoveClient(int watch, const
> unsigned char *
>
>  virNetlinkEventRemoveClientPrimitive(i);
>  VIR_DEBUG("removed client: %d by %s.",
> srv->handles[i].watch,
> -  srv->handles[i].watch ? "index" : "mac");
> +  srv->handles[i].watch ? "mac" : "index");


Aha! Actually it's not that it's switched, it's that I stupidly changed
the check to use srv->handles[i].watch (what's stored in the object)
instead of using watch (which came in the call args). I'll switch that
back and push.

Thanks for the fast test turnaround!


>  ret = 0;
>  goto cleanup;
>  }
> -- 
>
> Thanks for the effort !
>
> Dirk H
>
>

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


Re: [libvirt] [libvirt-glib 5/6] Remove now redundant 'path' property

2012-02-29 Thread Zeeshan Ali (Khattak)
On Wed, Feb 29, 2012 at 3:18 PM, Christophe Fergeau  wrote:
> On Tue, Feb 28, 2012 at 08:25:06PM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" 
>>
>> Remove now redundant 'path' property from GVirDomainDevice subclasses.
>> ---
>>  libvirt-gobject/libvirt-gobject-domain-disk.c      |   88 
>> 
>>  libvirt-gobject/libvirt-gobject-domain-disk.h      |    3 +-
>>  libvirt-gobject/libvirt-gobject-domain-interface.c |   89 
>> +++-
>>  libvirt-gobject/libvirt-gobject-domain-interface.h |    3 +-
>>  4 files changed, 31 insertions(+), 152 deletions(-)
>>
>> diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c 
>> b/libvirt-gobject/libvirt-gobject-domain-disk.c
>> index fb7672e..42e0e6c 100644
>> --- a/libvirt-gobject/libvirt-gobject-domain-disk.c
>> +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c
>> @@ -34,75 +34,22 @@
>>  #define GVIR_DOMAIN_DISK_GET_PRIVATE(obj)                         \
>>          (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_DISK, 
>> GVirDomainDiskPrivate))
>>
>> -struct _GVirDomainDiskPrivate
>> -{
>> -    gchar *path;
>> -};
>
> The rest of the code (GVirConfig*) always has a Private structure with a
> gboolean unused when it's empty, I'd rather we stayed consistent (though I
> keep thinking that we should remove these empty private structs everywhere
> :)

Ah ok, I didn't notice it. Since Daniel wants to keeep this private
struct too, I'll modify this patch accordingly.

>>  static GVirDomainDiskStats *
>> @@ -151,6 +82,15 @@ gvir_domain_disk_stats_free(GVirDomainDiskStats *stats)
>>  G_DEFINE_BOXED_TYPE(GVirDomainDiskStats, gvir_domain_disk_stats,
>>                      gvir_domain_disk_stats_copy, 
>> gvir_domain_disk_stats_free)
>>
>> +static const gchar *gvir_domain_disk_get_path(GVirDomainDisk *self)
>> +{
>> +    GVirConfigDomainDevice *config;
>> +
>> +    config = gvir_domain_device_get_config(GVIR_DOMAIN_DEVICE(self));
>
> config needs to be unref'ed after use.

Oops!

>> +
>> +    return gvir_config_domain_disk_get_target_dev (GVIR_CONFIG_DOMAIN_DISK 
>> (config));
>
> and the return value would be non-const after the changes I mentioned in my
> previous review.

But you are wrong (see reply to that mail). :)

>> diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.h 
>> b/libvirt-gobject/libvirt-gobject-domain-interface.h
>> index 62848db..26b7d1c 100644
>> --- a/libvirt-gobject/libvirt-gobject-domain-interface.h
>> +++ b/libvirt-gobject/libvirt-gobject-domain-interface.h
>> @@ -59,7 +59,8 @@ struct _GVirDomainInterface
>>  {
>>      GVirDomainDevice parent;
>>
>> -    GVirDomainInterfacePrivate *priv;
>> +    /* In case we need a private struct in future */
>> +    gpointer padding[1];
>
> Can you send an updated version of this patch once you have fixed all of
> these things?

Sure, once we agree on the const issue. :)

-- 
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] [libvirt-glib 1/6] Getters for GVirConfigDomainInterface attributes

2012-02-29 Thread Christophe Fergeau
On Wed, Feb 29, 2012 at 03:44:20PM +0200, Zeeshan Ali (Khattak) wrote:
> On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau  
> wrote:
> > On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
> >> From: "Zeeshan Ali (Khattak)" 
> >>
> >> ---
> >>  libvirt-gconfig/libvirt-gconfig-domain-interface.c |   35 
> >> 
> >>  libvirt-gconfig/libvirt-gconfig-domain-interface.h |    4 ++
> >>  libvirt-gconfig/libvirt-gconfig.sym                |    4 ++
> >>  3 files changed, 43 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c 
> >> b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
> >> index 85cc194..61d35bd 100644
> >> --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c
> >> +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
> >> @@ -96,6 +96,41 @@ void 
> >> gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface
> >>                                                      "model", "type", 
> >> model);
> >>  }
> >>
> >> +const char 
> >> *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface 
> >> *interface)
> >
> > Unless I'm missing something, this should not be const (caller needs to
> > free the returned string).
> 
> String getters usually do return const in the gobject world[1] as
> opposed to object getters as one require allocation/de-allocation and
> the other only requires incrementing/decrementing a counter. Also is
> the fact that strings are readily usable so you can just make a call
> to the getter and thats it but objects are not usually readily usable
> in the sense that you have to pass it to to some other function to do
> something with it. Note that I'm just making educated guesses here as
> to why 'const' for strings makes more sense as to why this practice is
> followed.

Strings are const in the gobject world when they are owned by the object
and not by the caller of the getter (ie the caller doesn't need to free
them). They are non-const when the caller has to free them. In this case,
the strings need to be freed by the caller, thus "const" should not be
used.

Christophe


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

Re: [libvirt] [libvirt-glib 1/6] Getters for GVirConfigDomainInterface attributes

2012-02-29 Thread Zeeshan Ali (Khattak)
On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau  wrote:
> On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" 
>>
>> ---
>>  libvirt-gconfig/libvirt-gconfig-domain-interface.c |   35 
>> 
>>  libvirt-gconfig/libvirt-gconfig-domain-interface.h |    4 ++
>>  libvirt-gconfig/libvirt-gconfig.sym                |    4 ++
>>  3 files changed, 43 insertions(+), 0 deletions(-)
>>
>> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c 
>> b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> index 85cc194..61d35bd 100644
>> --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> @@ -96,6 +96,41 @@ void 
>> gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface
>>                                                      "model", "type", model);
>>  }
>>
>> +const char 
>> *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface 
>> *interface)
>
> Unless I'm missing something, this should not be const (caller needs to
> free the returned string).

String getters usually do return const in the gobject world[1] as
opposed to object getters as one require allocation/de-allocation and
the other only requires incrementing/decrementing a counter. Also is
the fact that strings are readily usable so you can just make a call
to the getter and thats it but objects are not usually readily usable
in the sense that you have to pass it to to some other function to do
something with it. Note that I'm just making educated guesses here as
to why 'const' for strings makes more sense as to why this practice is
followed.

>> +{
>> +    g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL);
>> +
>> +    return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface),
>> +                                            "target", "device");
>
> This is "dev", not "device"

OK

>> +}
>> +
>> +GVirConfigDomainInterfaceLinkState 
>> gvir_config_domain_interface_get_link_state(GVirConfigDomainInterface 
>> *interface)
>> +{
>> +    g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface),
>> +                         GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT);
>> +
>> +    return 
>> gvir_config_object_get_attribute_genum(GVIR_CONFIG_OBJECT(interface),
>> +                                                  "link", "state",
>> +                                                  
>> GVIR_CONFIG_TYPE_DOMAIN_INTERFACE_LINK_STATE,
>> +                                                  
>> GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT);
>> +}
>
> Have you tested this? Is it available for reading after having been set?
> The reason I'm asking is that libvirt documentation only talk about
> modifying it.

No, I haven't really tested this part. The only test I have done is
with the Boxes patch[2].

>> diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
>> b/libvirt-gconfig/libvirt-gconfig.sym
>> index 96ce58f..f91b8b0 100644
>> --- a/libvirt-gconfig/libvirt-gconfig.sym
>> +++ b/libvirt-gconfig/libvirt-gconfig.sym
>> @@ -145,6 +145,10 @@ LIBVIRT_GCONFIG_0.0.4 {
>>       gvir_config_domain_interface_set_link_state;
>>       gvir_config_domain_interface_set_mac;
>>       gvir_config_domain_interface_set_model;
>> +     gvir_config_domain_interface_get_ifname;
>> +     gvir_config_domain_interface_get_link_state;
>> +     gvir_config_domain_interface_get_mac;
>> +     gvir_config_domain_interface_get_model;
>
> The other sections in this file put getter and setter for the same property
> next to each other, I'd prefer if we stayed consistant:

Sure.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

[1] http://developer.gnome.org/gtk/2.24/GtkWindow.html#gtk-window-get-icon-name
 http://developer.gnome.org/gtk/2.24/GtkWindow.html#gtk-window-get-title
 http://developer.gnome.org/gio/stable/GFileInfo.html#g-file-info-get-name
 
http://developer.gnome.org/gio/stable/GFileInfo.html#g-file-info-get-display-name

 I can provide many many more examples for core gnome libs if you
like but I guess you get the point :)

[2] https://bugzilla.gnome.org/show_bug.cgi?id=670994

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

Re: [libvirt] [PATCH 1/1] complete netlink event integration

2012-02-29 Thread D. Herrendoerfer


On Feb 29, 2012, at 9:51 AM, Laine Stump wrote:

Since I found a couple other problems, but have made you suffer  
through

enough back and forth already, I've made some final suggested changes
myself, and am sending a diff patch as a response to this message
(differences from your latest version to mine), as well as doing a
repost of the original two patches with your latest changes and mine
squashed in.

Please 1) take a look at the changes in my diff patches , then 2) test
the updated versions of the full patches (I'm unable to test beyond
compiling), and send your ACK if they're okay. Once I have your ACK
back, I'll push (I promise, I won't find any new issues *this*  
time :-)




ACK.

The code tests out fine. One nit I found in the logs was that the  
removal

was indicated by the wrong method:

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index fd6f751..614f8da 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -509,7 +509,7 @@ virNetlinkEventRemoveClient(int watch, const  
unsigned char *


 virNetlinkEventRemoveClientPrimitive(i);
 VIR_DEBUG("removed client: %d by %s.", srv- 
>handles[i].watch,

-  srv->handles[i].watch ? "index" : "mac");
+  srv->handles[i].watch ? "mac" : "index");
 ret = 0;
 goto cleanup;
 }
--

Thanks for the effort !

Dirk H

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


Re: [libvirt] [libvirt-glib 6/6] Add gvir_domain_get_devices()

2012-02-29 Thread Christophe Fergeau
On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" 
> 
> Currently we only support existing DomainDevice implementations:
> DomainDisk and DomainInterface.
> ---
>  .../libvirt-gobject-domain-device-private.h|2 +
>  libvirt-gobject/libvirt-gobject-domain-device.c|   21 ++
>  libvirt-gobject/libvirt-gobject-domain.c   |   42 
> 
>  libvirt-gobject/libvirt-gobject-domain.h   |3 +
>  libvirt-gobject/libvirt-gobject.sym|1 +
>  5 files changed, 69 insertions(+), 0 deletions(-)
> 
> diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h 
> b/libvirt-gobject/libvirt-gobject-domain-device-private.h
> index 72c660e..292a2ac 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-device-private.h
> +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h
> @@ -24,6 +24,8 @@
>  
>  G_BEGIN_DECLS
>  
> +G_GNUC_INTERNAL GVirDomainDevice 
> *gvir_domain_device_new(GVirConfigDomainDevice *config,
> + GVirDomain *domain);
>  virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self);
>  
>  G_END_DECLS
> diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c 
> b/libvirt-gobject/libvirt-gobject-domain-device.c
> index 85879d2..0ec5dad 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-device.c
> +++ b/libvirt-gobject/libvirt-gobject-domain-device.c
> @@ -176,3 +176,24 @@ GVirDomain 
> *gvir_domain_device_get_domain(GVirDomainDevice *device) {
>  GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice 
> *device) {
>  return g_object_ref (device->priv->config);
>  }
> +
> +G_GNUC_INTERNAL GVirDomainDevice 
> *gvir_domain_device_new(GVirConfigDomainDevice *config,
> + GVirDomain *domain)

gvir_domain_new_device(GVirDomain *domain, GVirConfigDomainDevice *config)
maybe ?
Having the argument that is always non-NULL first feels better to me.

> +{
> +GType type;
> +
> +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), NULL);
> +
> +if (GVIR_CONFIG_IS_DOMAIN_DISK(config))
> +type = GVIR_TYPE_DOMAIN_DISK;
> +else if (GVIR_CONFIG_IS_DOMAIN_INTERFACE(config))
> +type = GVIR_TYPE_DOMAIN_INTERFACE;
> +else {
> +g_debug("Unknown device type: %s", G_OBJECT_TYPE_NAME(config));
> +return NULL;
> +}

I'd have a slight preference for something like

switch (G_OBJECT_TYPE(config)) {
case GVIR_CONFIG_TYPE_DOMAIN_DISK:
type = GVIR_TYPE_DOMAIN_DISK;
break;
...

but I'm fine with either version.

> +
> +return GVIR_DOMAIN_DEVICE(g_object_new(type,
> +   "config", config,
> +   "domain", domain, NULL));
> +}
> diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
> b/libvirt-gobject/libvirt-gobject-domain.c
> index 23ad882..3c66c9c 100644
> --- a/libvirt-gobject/libvirt-gobject-domain.c
> +++ b/libvirt-gobject/libvirt-gobject-domain.c
> @@ -29,6 +29,7 @@
>  #include "libvirt-glib/libvirt-glib.h"
>  #include "libvirt-gobject/libvirt-gobject.h"
>  #include "libvirt-gobject-compat.h"
> +#include "libvirt-gobject/libvirt-gobject-domain-device-private.h"
>  
>  #define GVIR_DOMAIN_GET_PRIVATE(obj) \
>  (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN, 
> GVirDomainPrivate))
> @@ -868,3 +869,44 @@ gboolean gvir_domain_get_saved(GVirDomain *dom)
>  
>  return virDomainHasManagedSaveImage(dom->priv->handle, 0) == 1;
>  }
> +
> +/**
> + * gvir_domain_get_devices:
> + * @domain: the domain
> + * @err: place-holder for possible errors, or NULL
> + *
> + * Gets the list of devices attached to @domain
> + *
> + * Returns: (element-type LibvirtGObject.DomainDevice) (transfer full): a 
> newly
> + * allocated #GList of #GVirDomainDevice.
> + */
> +GList *gvir_domain_get_devices(GVirDomain *domain,
> +   GError **err)
> +{
> +GVirConfigDomain *config;
> +GList *config_devices;
> +GList *node;
> +GList *ret = NULL;
> +
> +g_return_val_if_fail(GVIR_IS_DOMAIN(domain), NULL);
> +
> +config = gvir_domain_get_config(domain, 0, err);
> +if (config == NULL)
> +return ret;
> +
> +config_devices = gvir_config_domain_get_devices(config);
> +for (node = config_devices; node != NULL; node = node->next) {
> +GVirConfigDomainDevice *device_config;
> +GVirDomainDevice *device;
> +
> +device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data);
> +device = gvir_domain_device_new(device_config, domain);
> +if (device != NULL)
> + ret = g_list_append(ret, device);

slight preference for g_list_prepend (much more efficient when the list is
big).
> +
> +g_object_unref (device_config);
> +}
> +g_list_free (config_devices);

The individual elements need t

Re: [libvirt] [libvirt-glib 5/6] Remove now redundant 'path' property

2012-02-29 Thread Daniel P. Berrange
On Wed, Feb 29, 2012 at 02:18:31PM +0100, Christophe Fergeau wrote:
> On Tue, Feb 28, 2012 at 08:25:06PM +0200, Zeeshan Ali (Khattak) wrote:
> > From: "Zeeshan Ali (Khattak)" 
> > 
> > Remove now redundant 'path' property from GVirDomainDevice subclasses.
> > ---
> >  libvirt-gobject/libvirt-gobject-domain-disk.c  |   88 
> > 
> >  libvirt-gobject/libvirt-gobject-domain-disk.h  |3 +-
> >  libvirt-gobject/libvirt-gobject-domain-interface.c |   89 
> > +++-
> >  libvirt-gobject/libvirt-gobject-domain-interface.h |3 +-
> >  4 files changed, 31 insertions(+), 152 deletions(-)
> > 
> > diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c 
> > b/libvirt-gobject/libvirt-gobject-domain-disk.c
> > index fb7672e..42e0e6c 100644
> > --- a/libvirt-gobject/libvirt-gobject-domain-disk.c
> > +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c
> > @@ -34,75 +34,22 @@
> >  #define GVIR_DOMAIN_DISK_GET_PRIVATE(obj) \
> >  (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_DISK, 
> > GVirDomainDiskPrivate))
> >  
> > -struct _GVirDomainDiskPrivate
> > -{
> > -gchar *path;
> > -};
> 
> The rest of the code (GVirConfig*) always has a Private structure with a
> gboolean unused when it's empty, I'd rather we stayed consistent (though I
> keep thinking that we should remove these empty private structs everywhere
> :)

[snip]

> > diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.h 
> > b/libvirt-gobject/libvirt-gobject-domain-interface.h
> > index 62848db..26b7d1c 100644
> > --- a/libvirt-gobject/libvirt-gobject-domain-interface.h
> > +++ b/libvirt-gobject/libvirt-gobject-domain-interface.h
> > @@ -59,7 +59,8 @@ struct _GVirDomainInterface
> >  {
> >  GVirDomainDevice parent;
> >  
> > -GVirDomainInterfacePrivate *priv;
> > +/* In case we need a private struct in future */
> > +gpointer padding[1];

I prefer that we keep the empty private struct (with a dummy gboolean unsed),
rather than doing this.

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] [libvirt-glib 5/6] Remove now redundant 'path' property

2012-02-29 Thread Christophe Fergeau
On Tue, Feb 28, 2012 at 08:25:06PM +0200, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" 
> 
> Remove now redundant 'path' property from GVirDomainDevice subclasses.
> ---
>  libvirt-gobject/libvirt-gobject-domain-disk.c  |   88 
> 
>  libvirt-gobject/libvirt-gobject-domain-disk.h  |3 +-
>  libvirt-gobject/libvirt-gobject-domain-interface.c |   89 
> +++-
>  libvirt-gobject/libvirt-gobject-domain-interface.h |3 +-
>  4 files changed, 31 insertions(+), 152 deletions(-)
> 
> diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c 
> b/libvirt-gobject/libvirt-gobject-domain-disk.c
> index fb7672e..42e0e6c 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-disk.c
> +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c
> @@ -34,75 +34,22 @@
>  #define GVIR_DOMAIN_DISK_GET_PRIVATE(obj) \
>  (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_DISK, 
> GVirDomainDiskPrivate))
>  
> -struct _GVirDomainDiskPrivate
> -{
> -gchar *path;
> -};

The rest of the code (GVirConfig*) always has a Private structure with a
gboolean unused when it's empty, I'd rather we stayed consistent (though I
keep thinking that we should remove these empty private structs everywhere
:)

> -
>  G_DEFINE_TYPE(GVirDomainDisk, gvir_domain_disk, GVIR_TYPE_DOMAIN_DEVICE);
>  
> -enum {
> -PROP_0,
> -PROP_PATH,
> -};
> -
>  #define GVIR_DOMAIN_DISK_ERROR gvir_domain_disk_error_quark()
>  
> -
>  static GQuark
>  gvir_domain_disk_error_quark(void)
>  {
>  return g_quark_from_static_string("gvir-domain-disk");
>  }
>  
> -static void gvir_domain_disk_get_property(GObject *object,
> -  guint prop_id,
> -  GValue *value,
> -  GParamSpec *pspec)
> -{
> -GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
> -GVirDomainDiskPrivate *priv = self->priv;
> -
> -switch (prop_id) {
> -case PROP_PATH:
> -g_value_set_string(value, priv->path);
> -break;
> -
> -default:
> -G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> -}
> -}
> -
> -
> -static void gvir_domain_disk_set_property(GObject *object,
> -  guint prop_id,
> -  const GValue *value,
> -  GParamSpec *pspec)
> -{
> -GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
> -GVirDomainDiskPrivate *priv = self->priv;
> -
> -switch (prop_id) {
> -case PROP_PATH:
> -g_free(priv->path);
> -priv->path = g_value_dup_string(value);
> -break;
> -
> -default:
> -G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> -}
> -}
> -
> -
>  static void gvir_domain_disk_finalize(GObject *object)
>  {
>  GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
> -GVirDomainDiskPrivate *priv = self->priv;
>  
>  g_debug("Finalize GVirDomainDisk=%p", self);
>  
> -g_free(priv->path);
> -
>  G_OBJECT_CLASS(gvir_domain_disk_parent_class)->finalize(object);
>  }
>  
> @@ -111,27 +58,11 @@ static void 
> gvir_domain_disk_class_init(GVirDomainDiskClass *klass)
>  GObjectClass *object_class = G_OBJECT_CLASS (klass);
>  
>  object_class->finalize = gvir_domain_disk_finalize;
> -object_class->get_property = gvir_domain_disk_get_property;
> -object_class->set_property = gvir_domain_disk_set_property;
> -
> -g_object_class_install_property(object_class,
> -PROP_PATH,
> -g_param_spec_string("path",
> -"Path",
> -"The disk path",
> -NULL,
> -G_PARAM_READWRITE |
> -
> G_PARAM_CONSTRUCT_ONLY |
> -
> G_PARAM_STATIC_STRINGS));
> -
> -g_type_class_add_private(klass, sizeof(GVirDomainDiskPrivate));

This would need to stay if we keep the Private struct

>  }
>  
>  static void gvir_domain_disk_init(GVirDomainDisk *self)
>  {
>  g_debug("Init GVirDomainDisk=%p", self);
> -
> -self->priv = GVIR_DOMAIN_DISK_GET_PRIVATE(self);

Ditto.

>  }
>  
>  static GVirDomainDiskStats *
> @@ -151,6 +82,15 @@ gvir_domain_disk_stats_free(GVirDomainDiskStats *stats)
>  G_DEFINE_BOXED_TYPE(GVirDomainDiskStats, gvir_domain_disk_stats,
>  gvir_domain_disk_stats_copy, gvir_domain_disk_stats_free)
>  
> +static const gchar *gvir_domain_disk_get_path(GVirDomainDisk *self)
> +{
> +GVirConfigDomainDevice *config;
> +
> +config = gvir_domain_device_get_config(GVIR_DOMAIN_DEVICE(self));

config needs to be unref'ed after use.
> +
> +  

Re: [libvirt] [libvirt-glib 4/6] Add 'config' property to GVirDomainDevice

2012-02-29 Thread Christophe Fergeau
The code to add the property looks good to me except for one minor nit, see
below.
I'd rather to see patch 6/6 come right after it since I was wondering how
config was getting set.

On Tue, Feb 28, 2012 at 08:25:05PM +0200, Zeeshan Ali (Khattak) wrote:
> @@ -144,3 +166,13 @@ virDomainPtr 
> gvir_domain_device_get_domain_handle(GVirDomainDevice *self)
>  GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) {
>  return g_object_ref (device->priv->domain);
>  }
> +
> +/**
> + * gvir_domain_device_get_config:
> + * @device: the domain device
> + *
> + * Returns: (transfer full): the config
> + */
> +GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice 
> *device) {

{ on a new line

Christophe


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

Re: [libvirt] [libvirt-glib 3/6] Add gvir_domain_device_get_domain()

2012-02-29 Thread Christophe Fergeau
On Wed, Feb 29, 2012 at 02:02:08PM +0100, Christophe Fergeau wrote:
> On Tue, Feb 28, 2012 at 08:25:04PM +0200, Zeeshan Ali (Khattak) wrote:
> > From: "Zeeshan Ali (Khattak)" 
> > 
> > Getter for the associated domain of a domain device.
> 
> NB: it already exists as a gobject property
> 
> > ---
> >  libvirt-gobject/libvirt-gobject-domain-device.c |   10 ++
> >  libvirt-gobject/libvirt-gobject-domain-device.h |3 +++
> >  libvirt-gobject/libvirt-gobject.sym |1 +
> >  3 files changed, 14 insertions(+), 0 deletions(-)
> > 
> > diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c 
> > b/libvirt-gobject/libvirt-gobject-domain-device.c
> > index 528b513..6282d8b 100644
> > --- a/libvirt-gobject/libvirt-gobject-domain-device.c
> > +++ b/libvirt-gobject/libvirt-gobject-domain-device.c
> > @@ -134,3 +134,13 @@ virDomainPtr 
> > gvir_domain_device_get_domain_handle(GVirDomainDevice *self)
> >  
> >  return handle;
> >  }
> > +
> > +/**
> > + * gvir_domain_device_get_domain:
> > + * @device: the domain device
> > + *
> > + * Returns: (transfer full): the associate domain
> 
> associated
> 
> ACK

Oh, one more nit below
> > + */
> > +GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) {

opening brace on a new line

> > +return g_object_ref (device->priv->domain);
> > +}
> > diff --git a/libvirt-gobject/libvirt-gobject-domain-device.h 
> > b/libvirt-gobject/libvirt-gobject-domain-device.h
> > index 96c0433..98acc2d 100644
> > --- a/libvirt-gobject/libvirt-gobject-domain-device.h
> > +++ b/libvirt-gobject/libvirt-gobject-domain-device.h
> > @@ -27,6 +27,8 @@
> >  #ifndef __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__
> >  #define __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__
> >  
> > +#include 
> > +
> >  G_BEGIN_DECLS
> >  
> >  #define GVIR_TYPE_DOMAIN_DEVICE(gvir_domain_device_get_type ())
> > @@ -58,6 +60,7 @@ struct _GVirDomainDeviceClass
> >  
> >  
> >  GType gvir_domain_device_get_type(void);
> > +GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device);
> >  
> >  G_END_DECLS
> >  
> > diff --git a/libvirt-gobject/libvirt-gobject.sym 
> > b/libvirt-gobject/libvirt-gobject.sym
> > index 5081f41..0097692 100644
> > --- a/libvirt-gobject/libvirt-gobject.sym
> > +++ b/libvirt-gobject/libvirt-gobject.sym
> > @@ -33,6 +33,7 @@ LIBVIRT_GOBJECT_0.0.4 {
> > gvir_connection_get_node_info;
> >  
> > gvir_domain_device_get_type;
> > +   gvir_domain_device_get_domain;
> >  
> > gvir_domain_disk_get_type;
> > gvir_domain_disk_stats_get_type;
> > -- 
> > 1.7.7.6
> > 
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list



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



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

Re: [libvirt] [libvirt-glib 3/6] Add gvir_domain_device_get_domain()

2012-02-29 Thread Christophe Fergeau
On Tue, Feb 28, 2012 at 08:25:04PM +0200, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" 
> 
> Getter for the associated domain of a domain device.

NB: it already exists as a gobject property

> ---
>  libvirt-gobject/libvirt-gobject-domain-device.c |   10 ++
>  libvirt-gobject/libvirt-gobject-domain-device.h |3 +++
>  libvirt-gobject/libvirt-gobject.sym |1 +
>  3 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c 
> b/libvirt-gobject/libvirt-gobject-domain-device.c
> index 528b513..6282d8b 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-device.c
> +++ b/libvirt-gobject/libvirt-gobject-domain-device.c
> @@ -134,3 +134,13 @@ virDomainPtr 
> gvir_domain_device_get_domain_handle(GVirDomainDevice *self)
>  
>  return handle;
>  }
> +
> +/**
> + * gvir_domain_device_get_domain:
> + * @device: the domain device
> + *
> + * Returns: (transfer full): the associate domain

associated

ACK

Christophe

> + */
> +GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) {
> +return g_object_ref (device->priv->domain);
> +}
> diff --git a/libvirt-gobject/libvirt-gobject-domain-device.h 
> b/libvirt-gobject/libvirt-gobject-domain-device.h
> index 96c0433..98acc2d 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-device.h
> +++ b/libvirt-gobject/libvirt-gobject-domain-device.h
> @@ -27,6 +27,8 @@
>  #ifndef __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__
>  #define __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__
>  
> +#include 
> +
>  G_BEGIN_DECLS
>  
>  #define GVIR_TYPE_DOMAIN_DEVICE(gvir_domain_device_get_type ())
> @@ -58,6 +60,7 @@ struct _GVirDomainDeviceClass
>  
>  
>  GType gvir_domain_device_get_type(void);
> +GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device);
>  
>  G_END_DECLS
>  
> diff --git a/libvirt-gobject/libvirt-gobject.sym 
> b/libvirt-gobject/libvirt-gobject.sym
> index 5081f41..0097692 100644
> --- a/libvirt-gobject/libvirt-gobject.sym
> +++ b/libvirt-gobject/libvirt-gobject.sym
> @@ -33,6 +33,7 @@ LIBVIRT_GOBJECT_0.0.4 {
>   gvir_connection_get_node_info;
>  
>   gvir_domain_device_get_type;
> + gvir_domain_device_get_domain;
>  
>   gvir_domain_disk_get_type;
>   gvir_domain_disk_stats_get_type;
> -- 
> 1.7.7.6
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


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

Re: [libvirt] [libvirt-glib 2/6] Add gvir_domain_disk_resize()

2012-02-29 Thread Christophe Fergeau
ACK
On Tue, Feb 28, 2012 at 08:25:03PM +0200, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" 
> 
> Binding for virDomainBlockResize().
> ---
>  libvirt-gobject/libvirt-gobject-domain-disk.c |   38 
> +
>  libvirt-gobject/libvirt-gobject-domain-disk.h |4 ++
>  libvirt-gobject/libvirt-gobject.sym   |1 +
>  3 files changed, 43 insertions(+), 0 deletions(-)
> 
> diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c 
> b/libvirt-gobject/libvirt-gobject-domain-disk.c
> index f98d816..fb7672e 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-disk.c
> +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c
> @@ -192,3 +192,41 @@ end:
>  virDomainFree(handle);
>  return ret;
>  }
> +
> +/**
> + * gvir_domain_disk_resize:
> + * @self: the domain disk
> + * @size: new size of the block image in kilobytes
> + * @flags: flags, currently unused. Pass '0'.
> + * @err: placeholder for an error, or NULL
> + *
> + * This function resize the disk of a running domain.
> + *
> + * Returns: TRUE if size was successfully changed, FALSE otherwise.
> + **/
> +gboolean gvir_domain_disk_resize(GVirDomainDisk *self,
> + guint64 size,
> + guint flags,
> + GError **err)
> +{
> +gboolean ret = FALSE;
> +virDomainPtr handle;
> +
> +g_return_val_if_fail(GVIR_IS_DOMAIN_DISK(self), FALSE);
> +g_return_val_if_fail(err == NULL || *err != NULL, FALSE);
> +
> +handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self));
> +
> +if (virDomainBlockResize(handle, self->priv->path, size, flags) < 0) {
> +gvir_set_error_literal(err, GVIR_DOMAIN_DISK_ERROR,
> +   0,
> +   "Failed to resize domain disk");
> +goto end;
> +}
> +
> +ret = TRUE;
> +
> +end:
> +virDomainFree(handle);
> +return ret;
> +}
> diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.h 
> b/libvirt-gobject/libvirt-gobject-domain-disk.h
> index 21f2357..1788d63 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-disk.h
> +++ b/libvirt-gobject/libvirt-gobject-domain-disk.h
> @@ -72,6 +72,10 @@ GType gvir_domain_disk_get_type(void);
>  GType gvir_domain_disk_stats_get_type(void);
>  
>  GVirDomainDiskStats *gvir_domain_disk_get_stats(GVirDomainDisk *self, GError 
> **err);
> +gboolean gvir_domain_disk_resize(GVirDomainDisk *self,
> + guint64 size,
> + guint flags,
> + GError **err);
>  
>  G_END_DECLS
>  
> diff --git a/libvirt-gobject/libvirt-gobject.sym 
> b/libvirt-gobject/libvirt-gobject.sym
> index 1ad6b53..5081f41 100644
> --- a/libvirt-gobject/libvirt-gobject.sym
> +++ b/libvirt-gobject/libvirt-gobject.sym
> @@ -37,6 +37,7 @@ LIBVIRT_GOBJECT_0.0.4 {
>   gvir_domain_disk_get_type;
>   gvir_domain_disk_stats_get_type;
>   gvir_domain_disk_get_stats;
> + gvir_domain_disk_resize;
>  
>   gvir_domain_interface_get_type;
>   gvir_domain_interface_stats_get_type;
> -- 
> 1.7.7.6
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


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

[libvirt] [PATCH v2 2/2] Allow x86 to fetch sysinfo from /proc/cpuinfo when dmidecode is absent.

2012-02-29 Thread Prerna
>From 98c4f68702bc21060347011144603ee10be4847e Mon Sep 17 00:00:00 2001
From: Prerna Saxena 
Date: Thu, 16 Feb 2012 15:33:43 +0530
Subject: [PATCH 2/2] Sysinfo : Allow x86 to fetch sysinfo from /proc/cpuinfo
 in the event 'dmidecode' is absent in the system.

Based on Eric's suggestion
(http://www.redhat.com/archives/libvir-list/2012-February/msg00509.html)

Changelog:
-
 Cleanups

---
 src/util/sysinfo.c |   97 +--
 1 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index 78efc32..290b69f 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -598,6 +598,98 @@ no_memory:
 return -1;
 }
 
+/* If a call to 'dmidecode' fails,
+ * extract basic sysinfo from /proc/cpuinfo */
+
+static int
+virSysinfoParseCPUInfoProcessor(const char *base, virSysinfoDefPtr ret)
+{
+const char *cur;
+char *eol, *tmp_base;
+virSysinfoProcessorDefPtr processor;
+
+while((tmp_base = strstr(base, "processor")) != NULL) {
+base = tmp_base;
+eol = strchr(base, '\n');
+cur = strchr(base, ':') + 1;
+
+if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) {
+goto no_memory;
+}
+
+processor = &ret->processor[ret->nprocessor - 1];
+virSkipSpaces(&cur);
+if (eol &&
+(processor->processor_socket_destination =
+   strndup(cur, eol - cur)) == NULL)
+goto no_memory;
+
+
+if ((cur = strstr(base, "vendor_id")) != NULL) {
+cur = strchr(cur, ':') + 1;
+eol = strchr(cur, '\n');
+virSkipSpaces(&cur);
+if (eol &&
+   ((processor->processor_manufacturer =
+  strndup(cur, eol - cur)) == NULL))
+goto no_memory;
+}
+
+if ((cur = strstr(base, "cpu family")) != NULL) {
+cur = strchr(cur, ':') + 1;
+eol = strchr(cur, '\n');
+virSkipSpaces(&cur);
+if (eol &&
+((processor->processor_family = strndup(cur, eol - cur))
+   == NULL))
+goto no_memory;
+}
+
+if ((cur = strstr(base, "model name")) != NULL) {
+cur = strchr(cur, ':') + 1;
+eol = strchr(cur, '\n');
+virSkipSpaces(&cur);
+if (eol &&
+   ((processor->processor_type = strndup(cur, eol - cur))
+ == NULL))
+goto no_memory;
+}
+
+base = cur;
+}
+
+return 0;
+
+no_memory:
+return -1;
+}
+
+static virSysinfoDefPtr
+virCPUInfoSysinfoRead(void) {
+virSysinfoDefPtr ret = NULL;
+char *outbuf = NULL;
+
+if (VIR_ALLOC(ret) < 0)
+goto no_memory;
+
+if(virFileReadAll(CPUINFO, 2, &outbuf) < 0) {
+virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to open %s"), CPUINFO);
+return NULL;
+}
+
+ret->nprocessor = 0;
+ret->processor = NULL;
+if (virSysinfoParseCPUInfoProcessor(outbuf, ret) < 0)
+goto no_memory;
+
+return ret;
+
+no_memory:
+VIR_FREE(outbuf);
+return NULL;
+}
+
 virSysinfoDefPtr
 virSysinfoRead(void) {
 char *path;
@@ -607,10 +699,7 @@ virSysinfoRead(void) {
 
 path = virFindFileInPath(SYSINFO_SMBIOS_DECODER);
 if (path == NULL) {
-virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to find path for %s binary"),
- SYSINFO_SMBIOS_DECODER);
-return NULL;
+return virCPUInfoSysinfoRead();
 }
 
 cmd = virCommandNewArgList(path, "-q", "-t", "0,1,4,17", NULL);
-- 
1.7.7



-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India


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


Re: [libvirt] [libvirt-glib 1/6] Getters for GVirConfigDomainInterface attributes

2012-02-29 Thread Christophe Fergeau
On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" 
> 
> ---
>  libvirt-gconfig/libvirt-gconfig-domain-interface.c |   35 
> 
>  libvirt-gconfig/libvirt-gconfig-domain-interface.h |4 ++
>  libvirt-gconfig/libvirt-gconfig.sym|4 ++
>  3 files changed, 43 insertions(+), 0 deletions(-)
> 
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c 
> b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
> index 85cc194..61d35bd 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
> @@ -96,6 +96,41 @@ void 
> gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface
>  "model", "type", model);
>  }
>  
> +const char 
> *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface *interface)

Unless I'm missing something, this should not be const (caller needs to
free the returned string).

> +{
> +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL);
> +
> +return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface),
> +"target", "device");

This is "dev", not "device"

> +}
> +
> +GVirConfigDomainInterfaceLinkState 
> gvir_config_domain_interface_get_link_state(GVirConfigDomainInterface 
> *interface)
> +{
> +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface),
> + GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT);
> +
> +return 
> gvir_config_object_get_attribute_genum(GVIR_CONFIG_OBJECT(interface),
> +  "link", "state",
> +  
> GVIR_CONFIG_TYPE_DOMAIN_INTERFACE_LINK_STATE,
> +  
> GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT);
> +}

Have you tested this? Is it available for reading after having been set?
The reason I'm asking is that libvirt documentation only talk about
modifying it.

> +
> +const char *gvir_config_domain_interface_get_mac(GVirConfigDomainInterface 
> *interface)

Non-const here too

> +{
> +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL);
> +
> +return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface),
> +"mac", "address");
> +}
> +
> +const char *gvir_config_domain_interface_get_model(GVirConfigDomainInterface 
> *interface)

And same for this getter.

> +{
> +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL);
> +
> +return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface),
> +"model", "type");
> +}
> +
>  G_GNUC_INTERNAL GVirConfigDomainDevice *
>  gvir_config_domain_interface_new_from_tree(GVirConfigXmlDoc *doc,
> xmlNodePtr tree)
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.h 
> b/libvirt-gconfig/libvirt-gconfig-domain-interface.h
> index 6e802fb..c8c4fb3 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.h
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.h
> @@ -72,6 +72,10 @@ void 
> gvir_config_domain_interface_set_mac(GVirConfigDomainInterface *interface,
>const char *mac_address);
>  void gvir_config_domain_interface_set_model(GVirConfigDomainInterface 
> *interface,
>  const char *model);
> +const char 
> *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface 
> *interface);
> +GVirConfigDomainInterfaceLinkState 
> gvir_config_domain_interface_get_link_state(GVirConfigDomainInterface 
> *interface);
> +const char *gvir_config_domain_interface_get_mac(GVirConfigDomainInterface 
> *interface);
> +const char *gvir_config_domain_interface_get_model(GVirConfigDomainInterface 
> *interface);
>  
>  G_END_DECLS
>  
> diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
> b/libvirt-gconfig/libvirt-gconfig.sym
> index 96ce58f..f91b8b0 100644
> --- a/libvirt-gconfig/libvirt-gconfig.sym
> +++ b/libvirt-gconfig/libvirt-gconfig.sym
> @@ -145,6 +145,10 @@ LIBVIRT_GCONFIG_0.0.4 {
>   gvir_config_domain_interface_set_link_state;
>   gvir_config_domain_interface_set_mac;
>   gvir_config_domain_interface_set_model;
> + gvir_config_domain_interface_get_ifname;
> + gvir_config_domain_interface_get_link_state;
> + gvir_config_domain_interface_get_mac;
> + gvir_config_domain_interface_get_model;

The other sections in this file put getter and setter for the same property
next to each other, I'd prefer if we stayed consistant:

+   gvir_config_domain_interface_get_ifname;
+   gvir_config_domain_interface_set_ifname;
+   gvir_config_domain_interface_get_link_state;
gvir_config_domain_interface_set_l

Re: [libvirt] [PATCH 2/2] qemu: Add pre-migration hook

2012-02-29 Thread Jiri Denemark
On Tue, Feb 28, 2012 at 17:04:29 -0700, Eric Blake wrote:
> On 02/28/2012 02:49 PM, Jiri Denemark wrote:
> > This hook is called during the Prepare phase on destination host and may
> > be used for changing domain XML.
> > ---
> >  docs/hooks.html.in|   35 +++
> >  src/qemu/qemu_migration.c |   40 
> >  src/util/hooks.c  |3 ++-
> >  src/util/hooks.h  |1 +
> >  4 files changed, 66 insertions(+), 13 deletions(-)
...
> > @@ -1150,6 +1152,43 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
> >  goto cleanup;
> >  }
> >  
> > +/* Let migration hook filter domain XML */
> > +if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
> > +char *xml = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE);
> > +int hookret;
> > +
> > +hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, def->name,
> > +  VIR_HOOK_QEMU_OP_MIGRATE, 
> > VIR_HOOK_SUBOP_BEGIN,
> > +  NULL, xml, &xmlout);
> 
> Needs to check for xml being NULL on OOM before virHookCall.

Oops, I just copy&pasted the code from other places, where domain XML is
probably not so important so we don't care if the hook gets it or not.

> ACK with those issues fixed.

I fixed the issues and pushed this small series. Thanks.

Jirka

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


[libvirt] [PATCH v2 1/2] Implement sysinfo on PowerPC.

2012-02-29 Thread Prerna
From: Prerna Saxena 
Date: Tue, 7 Feb 2012 16:55:26 +0530
Subject: [PATCH 1/2] Implement sysinfo on PowerPC.

Libvirt on x86 parses 'dmidecode' to gather characteristics of host
system, which are then reflected to libvirt users by virSysinfoRead(),
invoked by 'virsh sysinfo'.
This patch implements it on PowerPC by reading /proc/cpuinfo.

The presently available fields in 'sysinfo' are strongly tied to
dmidecode output fields. This patch attempts to retrofit the
information available in PowerPC to appropriate sysinfo fields. I will
be happy to change the organization of this information to if there
are expected outputs for individual fields. (I couldnt find any
documentation which explained what each sysinfo field was expected
to convey.)

TODOS:
1. Adding Memory DIMM information
2) Firmware () details.
3) Expand  tag to have more fields available.

Example output on PowerPC :
virsh # sysinfo

  
PowerNV 8246-L2C
8246-L2C
PowerNV
  
  
0
POWER7 (raw), altivec supported
2.3 (pvr 003f 0203)
  
  
4
POWER7 (raw), altivec supported
2.3 (pvr 003f 0203)
  
  ..

Changelog:
-
Cleanups.

---
 src/util/sysinfo.c |  133 +++-
 1 files changed, 131 insertions(+), 2 deletions(-)

diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index 39c7581..78efc32 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -44,6 +44,7 @@
  __FUNCTION__, __LINE__, __VA_ARGS__)
 
 #define SYSINFO_SMBIOS_DECODER "dmidecode"
+#define CPUINFO "/proc/cpuinfo"
 
 VIR_ENUM_IMPL(virSysinfo, VIR_SYSINFO_LAST,
   "smbios");
@@ -113,10 +114,138 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
  *
  * Returns: a filled up sysinfo structure or NULL in case of error
  */
-#if defined(WIN32) || \
+
+#if defined(__powerpc__)
+static int
+virSysinfoParseSystem(const char *base, virSysinfoDefPtr ret)
+{
+char *eol = NULL;
+const char *cur;
+
+if ((cur = strstr(base, "platform")) == NULL)
+return 0;
+
+base = cur;
+/* Account for format 'platform: '*/
+cur = strchr(cur, ':') + 1;
+eol = strchr(cur, '\n');
+virSkipSpaces(&cur);
+if (eol &&
+   ((ret->system_family = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+
+if ((cur = strstr(base, "model")) != NULL) {
+cur = strchr(cur, ':') + 1;
+eol = strchr(cur, '\n');
+virSkipSpaces(&cur);
+if (eol && ((ret->system_serial = strndup(cur, eol - cur))
+   == NULL))
+goto no_memory;
+}
+
+if ((cur = strstr(base, "machine")) != NULL) {
+cur = strchr(cur, ':') + 1;
+eol = strchr(cur, '\n');
+virSkipSpaces(&cur);
+if (eol && ((ret->system_version = strndup(cur, eol - cur))
+== NULL))
+goto no_memory;
+}
+
+return 0;
+
+no_memory:
+return -1;
+}
+
+static int
+virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret)
+{
+const char *cur;
+char *eol, *tmp_base;
+virSysinfoProcessorDefPtr processor;
+
+while((tmp_base = strstr(base, "processor")) != NULL) {
+base = tmp_base;
+eol = strchr(base, '\n');
+cur = strchr(base, ':') + 1;
+
+if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) {
+goto no_memory;
+}
+processor = &ret->processor[ret->nprocessor - 1];
+
+virSkipSpaces(&cur);
+if (eol &&
+((processor->processor_socket_destination = strndup
+ (cur, eol - cur)) == NULL))
+goto no_memory;
+
+if ((cur = strstr(base, "cpu")) != NULL) {
+cur = strchr(cur, ':') + 1;
+eol = strchr(cur, '\n');
+virSkipSpaces(&cur);
+if (eol &&
+   ((processor->processor_type = strndup(cur, eol - cur))
+ == NULL))
+goto no_memory;
+}
+
+if ((cur = strstr(base, "revision")) != NULL) {
+cur = strchr(cur, ':') + 1;
+eol = strchr(cur, '\n');
+virSkipSpaces(&cur);
+if (eol &&
+   ((processor->processor_version = strndup(cur, eol - cur))
+== NULL))
+goto no_memory;
+}
+
+base = cur;
+}
+
+return 0;
+
+no_memory:
+return -1;
+}
+
+/* virSysinfoRead for PowerPC
+ * Gathers sysinfo data from /proc/cpuinfo */
+virSysinfoDefPtr
+virSysinfoRead(void) {
+virSysinfoDefPtr ret = NULL;
+char *outbuf = NULL;
+
+if (VIR_ALLOC(ret) < 0)
+goto no_memory;
+
+if(virFileReadAll(CPUINFO, 2048, &outbuf) < 0) {
+virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to open %s"), CPUINFO);
+   

[libvirt] [PATCH] Fix typo in domain XML documentation

2012-02-29 Thread Christophe Fergeau
s/Modyfing/Modifying
---
 docs/formatdomain.html.in |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 5305f82..9e7dd08 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2580,7 +2580,7 @@ qemu-kvm -net nic,model=? /dev/null
   Since 0.9.4
 
 
-Modyfing virtual link state
+Modifying virtual link state
 
   ...
   
-- 
1.7.7.6

--
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-29 Thread Christophe Fergeau
On Tue, Feb 28, 2012 at 05:04:57PM +0100, Michal Privoznik wrote:
> On 24.02.2012 11:34, Christophe Fergeau wrote:
> >  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);
> 
> In fact, this needs to be wrapped with curly braces as the check for
> tlsPort != -1 is meant to protect virBufferAsprintf() in the first
> place. Sorry for not catching this earlier.

Ugh, sorry about that silly bug, thanks for catching it.

Christophe


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

Re: [libvirt] [PATCH 3/4] libvirt-guests: Check if URI is reachable before launching commands

2012-02-29 Thread Peter Krempa

On 02/29/2012 04:02 AM, Eric Blake wrote:

On 02/28/2012 11:00 AM, Peter Krempa wrote:

This patch adds a check to the libvirt-guests script to check for the
URI to be alive before attempting any calls. This avoids nasty error
messages and allows us to fail gracefuly and continue on other URIs


s/gracefuly/gracefully/


configured in the script.
---
  tools/libvirt-guests.init.sh |   24 +++-
  1 files changed, 19 insertions(+), 5 deletions(-)



ACK.


Thanks. I pushed patches 1,2 and 3 as they make sense without the last 
one and will send a fixed version soon.


Peter








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


Re: [libvirt] [PATCH] storage: fix typo

2012-02-29 Thread Michal Privoznik
On 29.02.2012 11:53, Daniel P. Berrange wrote:
> On Wed, Feb 29, 2012 at 11:44:47AM +0100, Michal Privoznik wrote:
>> * src/storage/storage_driver.c (storageVolumeWipeInternal):
>> s/ pfitzner33/pfitzner33/.
>> ---
>>  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 540e5d7..9130a40 100644
>> --- a/src/storage/storage_driver.c
>> +++ b/src/storage/storage_driver.c
>> @@ -1937,7 +1937,7 @@ storageVolumeWipeInternal(virStorageVolDefPtr def,
>>  alg_char = "pfitzner7";
>>  break;
>>  case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33:
>> -alg_char = " pfitzner33";
>> +alg_char = "pfitzner33";
>>  break;
>>  case VIR_STORAGE_VOL_WIPE_ALG_RANDOM:
>>  alg_char = "random";
> 
> ACK
> 
> Daniel

Thanks, pushed.

Michal

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


Re: [libvirt] [PATCH] storage: fix typo

2012-02-29 Thread Daniel P. Berrange
On Wed, Feb 29, 2012 at 11:44:47AM +0100, Michal Privoznik wrote:
> * src/storage/storage_driver.c (storageVolumeWipeInternal):
> s/ pfitzner33/pfitzner33/.
> ---
>  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 540e5d7..9130a40 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -1937,7 +1937,7 @@ storageVolumeWipeInternal(virStorageVolDefPtr def,
>  alg_char = "pfitzner7";
>  break;
>  case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33:
> -alg_char = " pfitzner33";
> +alg_char = "pfitzner33";
>  break;
>  case VIR_STORAGE_VOL_WIPE_ALG_RANDOM:
>  alg_char = "random";

ACK

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] storage: fix typo

2012-02-29 Thread Michal Privoznik
* src/storage/storage_driver.c (storageVolumeWipeInternal):
s/ pfitzner33/pfitzner33/.
---
 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 540e5d7..9130a40 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1937,7 +1937,7 @@ storageVolumeWipeInternal(virStorageVolDefPtr def,
 alg_char = "pfitzner7";
 break;
 case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33:
-alg_char = " pfitzner33";
+alg_char = "pfitzner33";
 break;
 case VIR_STORAGE_VOL_WIPE_ALG_RANDOM:
 alg_char = "random";
-- 
1.7.3.4

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


Re: [libvirt] [PATCH] qemu: Don't emit tls-port spice option if port is -1

2012-02-29 Thread Jiri Denemark
On Tue, Feb 28, 2012 at 17:17:36 +0100, Michal Privoznik wrote:
> On 28.02.2012 14:16, Jiri Denemark wrote:
> > Bug introduced by commit eda0fc7a.
> > ---
> >  src/qemu/qemu_command.c |9 ++---
> >  1 files changed, 6 insertions(+), 3 deletions(-)
> > 
> 
> ACK

Pushed, thanks.

Jirka

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


[libvirt] Fwd: Re: [libvirt-users] [need your help] Sys::Virt

2012-02-29 Thread Alex Jia

Hi all,
Anybody has the same experience? Evaggelos need your help.

Thanks & Regards,
Alex


 # uname -a
 Linux mylaptop 3.2.7-1-ARCH #1 SMP PREEMPT Tue Feb 21 16:59:04 UTC
 2012 i686 Intel(R) Core(TM)2 CPU T5600 @ 1.83GHz GenuineIntel
 GNU/Linux

 # perl -v
 This is perl 5, version 14, subversion 2 (v5.14.2) built for
 i686-linux-thread-multi

 # libvirt 0.9.10-1
 # libxml-enno-1.02
 # XML-XPathEngine-0.13
 # XML-DOM-XPath-0.14
 # CPAN-Changes-0.18

 # perl-test-pod-coverage 1.08-2
 # perl-time-hires 1.9725-1
 # perl-xml-xpath 1.13-6
 # perl-test-pod 1.45-1

 # gcc --version
 gcc (GCC) 4.6.2 20120120 (prerelease)

 Plz advice

 Evaggelos Balaskas - Unix System Engineer
 http://gr.linkedin.com/in/evaggelosbalaskas



 Original Message 
Subject:Re: [libvirt-users] Sys::Virt
Date:   Sun, 26 Feb 2012 21:59:16 -0500 (EST)
From:   Alex Jia 
To: ebalas...@ebalaskas.gr
CC: libvirt-us...@redhat.com



Hi Evaggelos,
I used latest libvirt-perl git tree, then compile it on RHEL,
however, I haven't met the issue, everything is okay for me,
so I want to know your platform and compilation environment.

Is it okay to compile latest libvirt-perl for you?
% git clone git://libvirt.org/libvirt-perl.git
% perl Makefile.PL
% make


- Original Message -
From: "Evaggelos Balaskas"
To: libvirt-us...@redhat.com
Sent: Sunday, February 26, 2012 12:59:30 AM
Subject: [libvirt-users] Sys::Virt

Can anyone help me:

$ perl Makefile.PL
Checking if your kit is complete...
Looks good
Writing Makefile for Sys::Virt
Writing MYMETA.yml and MYMETA.json

$ make
/usr/bin/perl "-Iblib/arch" "-Iblib/lib" perl-Sys-Virt.spec.PL
perl-Sys-Virt.spec
make: *** [perl-Sys-Virt.spec] Segmentation fault
make: *** Deleting file `perl-Sys-Virt.spec'

Evaggelos Balaskas - Unix System Engineer
http://gr.linkedin.com/in/evaggelosbalaskas

___
libvirt-users mailing list
libvirt-us...@redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-users

___
libvirt-users mailing list
libvirt-us...@redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-users

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

Re: [libvirt] [PATCH 3/3] virsh: expose partial pull

2012-02-29 Thread Laine Stump
On 02/18/2012 12:44 PM, Eric Blake wrote:
> Now virsh can call virDomainBlockRebase.

ACK.


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


Re: [libvirt] [PATCH 2/3] qemu: pass block pull backing file to monitor

2012-02-29 Thread Laine Stump
On 02/18/2012 12:44 PM, Eric Blake wrote:
> This actually wires up the new optional parameter to block_stream:
> http://wiki.qemu.org/Features/LiveBlockMigration/ImageStreamingAPI
>
> The error checking is still sparse, since libvirt must not use
> qemu-img or header probing on a qcow2 file in use by qemu to
> check if the backing file name is valid; so for now, libvirt is
> relying on qemu to diagnose an incorrect backing name.  Fixing this
> will require libvirt to track the entire backing file chain at the
> time qemu is started and keeps it updated with snapshot and pull
> operations.

Looks like mostly pushing the new parameter down the callstack, moving
the location (and conditions)
of the check for base being specified at an inappropriate time, and
changing the json command sent to actually use base in certain cases.

ACK.

>
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockJob): Add
> parameter, and update callers.
> * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockJob): Update
> signature.
> * src/qemu/qemu_monitor.h (qemuMonitorBlockJob): Likewise.
> * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Update caller.
> * src/qemu/qemu_monitor.c (qemuMonitorBlockJob): Likewise.
> ---
>  src/qemu/qemu_driver.c   |   15 ++-
>  src/qemu/qemu_monitor.c  |7 ---
>  src/qemu/qemu_monitor.h  |6 --
>  src/qemu/qemu_monitor_json.c |   26 +++---
>  src/qemu/qemu_monitor_json.h |3 ++-
>  5 files changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 717bdf1..8308b3b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11367,14 +11367,6 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char 
> *path, const char *base,
>  if (!device) {
>  goto cleanup;
>  }
> -/* XXX - add a qemu capability check; if qemu 1.1 or newer, then
> - *  validate and convert non-NULL base into something that can
> - * be passed as optional base argument.  */
> -if (base)  {
> -qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> -_("partial block pull is not supported with this 
> QEMU binary"));
> -goto cleanup;
> -}
>
>  if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
>  goto cleanup;
> @@ -11387,7 +11379,12 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char 
> *path, const char *base,
>
>  qemuDomainObjEnterMonitorWithDriver(driver, vm);
>  priv = vm->privateData;
> -ret = qemuMonitorBlockJob(priv->mon, device, bandwidth, info, mode);
> +/* XXX - add a qemu capability check, since only qemu 1.1 or newer
> + * supports the base argument.
> + * XXX - libvirt should really be tracking the backing file chain
> + * itself, and validating that base is on the chain, rather than
> + * relying on qemu to do this.  */
> +ret = qemuMonitorBlockJob(priv->mon, device, base, bandwidth, info, 
> mode);
>  qemuDomainObjExitMonitorWithDriver(driver, vm);
>
>  endjob:
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 903463d..70ef1d6 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2653,17 +2653,18 @@ int qemuMonitorScreendump(qemuMonitorPtr mon,
>
>  int qemuMonitorBlockJob(qemuMonitorPtr mon,
>  const char *device,
> +const char *base,
>  unsigned long bandwidth,
>  virDomainBlockJobInfoPtr info,
>  int mode)
>  {
>  int ret = -1;
>
> -VIR_DEBUG("mon=%p, device=%s, bandwidth=%lu, info=%p, mode=%o",
> -  mon, device, bandwidth, info, mode);
> +VIR_DEBUG("mon=%p, device=%s, base=%s, bandwidth=%lu, info=%p, mode=%o",
> +  mon, device, NULLSTR(base), bandwidth, info, mode);
>
>  if (mon->json)
> -ret = qemuMonitorJSONBlockJob(mon, device, bandwidth, info, mode);
> +ret = qemuMonitorJSONBlockJob(mon, device, base, bandwidth, info, 
> mode);
>  else
>  qemuReportError(VIR_ERR_INVALID_ARG, "%s",
>  _("block jobs require JSON monitor"));
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 7c6c52b..1d2caea 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1,7 +1,7 @@
>  /*
>   * qemu_monitor.h: interaction with QEMU monitor console
>   *
> - * Copyright (C) 2006-2011 Red Hat, Inc.
> + * Copyright (C) 2006-2012 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -518,9 +518,11 @@ typedef enum {
>
>  int qemuMonitorBlockJob(qemuMonitorPtr mon,
>  const char *device,
> +const char *back,
>  unsigned long bandwidth,
>  virDomainBlockJobInfoPtr info,
> -  

Re: [libvirt] [PATCH 1/3] qemu: require json for block jobs

2012-02-29 Thread Laine Stump
On 02/18/2012 12:44 PM, Eric Blake wrote:
> Block job commands are not part of upstream qemu until 1.1; and
> proper support of job completion and cancellation depends on being
> able to receive QMP events, which implies the JSON monitor.
> Additionally, some early versions of block job commands were
> backported to RHEL qemu, but these versions lacked asynchronous
> job cancellation and partial block pull, so there are several
> patches that will still be needed in this area of libvirt code
> to support both flavors of block job commands.
>
> Due to earlier patches in libvirt, we are guaranteed that all versions
> of qemu that support block job commands already require libvirt to
> use the JSON monitor.  That means that the text version of block jobs
> will not be used, and having to refactor two copies of the block job
> handlers makes no sense.  So instead, we delete the text handlers.
>
> * src/qemu/qemu_monitor.c (qemuMonitorBlockJob): Drop text monitor
> support.
> * src/qemu/qemu_monitor_text.h (qemuMonitorTextBlockJob): Delete.
> * src/qemu/qemu_monitor_text.c (qemuMonitorTextParseBlockJobOne)
> (qemuMonitorTextParseBlockJob, qemuMonitorTextBlockJob):
> Likewise.
> ---
>  src/qemu/qemu_monitor.c  |9 +-
>  src/qemu/qemu_monitor_text.c |  175 
> +-
>  src/qemu/qemu_monitor_text.h |8 +--
>  3 files changed, 7 insertions(+), 185 deletions(-)

I'm sure you were very pleased with yourself for creating a diffstat
like this :-)

The reasoning behind the removal makes sense to me. ACK.

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


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

2012-02-29 Thread Laine Stump
On 02/24/2012 07:39 PM, Eric Blake wrote:
> 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.

I just noticed this hasn't been ACKed/pushed yet.

Sounds like a good idea to me. ACK.

>
> * .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 *\(|\ + @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' \

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


Re: [libvirt] [PATCH] util: fix a typo

2012-02-29 Thread Alex Jia

On 02/29/2012 04:21 PM, Daniel P. Berrange wrote:

On Wed, Feb 29, 2012 at 02:54:38PM +0800, Alex Jia wrote:

* src/util/event_poll.c: (virEventPollRunOnce): s/imeout/timeout/.

Signed-off-by: Alex Jia
---
  src/util/event_poll.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/util/event_poll.c b/src/util/event_poll.c
index 30dec74..038e75f 100644
--- a/src/util/event_poll.c
+++ b/src/util/event_poll.c
@@ -615,7 +615,7 @@ int virEventPollRunOnce(void) {

   retry:
  PROBE(EVENT_POLL_RUN,
-  "nhandles=%d imeout=%d",
+  "nhandles=%d timeout=%d",
nfds, timeout);
  ret = poll(fds, nfds, timeout);
  if (ret<  0) {


ACK


Daniel

Thanks and push now.

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


Re: [libvirt] [PATCH] storage: fix a typo

2012-02-29 Thread Alex Jia

On 02/29/2012 04:20 PM, Daniel P. Berrange wrote:

On Wed, Feb 29, 2012 at 02:21:41PM +0800, Alex Jia wrote:

* src/storage/storage_driver.c (storageVolumeWipeInternal): s/shneier/schneier.

http://code.google.com/p/diskscrub/

Signed-off-by: Alex Jia
---
  src/storage/storage_driver.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

ACK


Thanks and push now.

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index df0e291..540e5d7 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1931,7 +1931,7 @@ storageVolumeWipeInternal(virStorageVolDefPtr def,
  alg_char = "gutmann";
  break;
  case VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER:
-alg_char = "shneier";
+alg_char = "schneier";
  break;
  case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7:
  alg_char = "pfitzner7";
--


Daniel


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


Re: [libvirt] [PATCH] docs: comments wiping supported algorithms

2012-02-29 Thread Alex Jia

On 02/29/2012 04:24 PM, Daniel P. Berrange wrote:

On Wed, Feb 29, 2012 at 04:02:59PM +0800, Alex Jia wrote:

The current scrub version doesn't support pfitzner7, pfitzner33 and schneier 
patterns
on RHEL, we should comment it in virsh man page.

* tools/virsh.pod: update wiping algorithms docs.

Signed-off-by: Alex Jia
---
  tools/virsh.pod |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index c306a38..eba9389 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2081,6 +2081,8 @@ B
pfitzner33 - Roy Pfitzner's 33-random-pass method: random x33.
random - 1-pass pattern: random.

+B: Not all algorithms of wiping are expected to work, it depends on concrete 
B  version.

I'd word this slightly differently:

   B  the availability of algorithms may be limited by the version
   of the C  binary installed on the host.
I'd like this:-) thanks, I will follow the above modification and 
directly push.

Regards,
Daniel


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


[libvirt] [PATCHv7 2/2] Add de-association handling to macvlan code

2012-02-29 Thread Laine Stump
From: "D. Herrendoerfer" 

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 
---
 src/qemu/qemu_migration.c|2 +-
 src/util/virnetdevmacvlan.c  |  360 +-
 src/util/virnetdevvportprofile.c |   33 +++-
 src/util/virnetdevvportprofile.h |3 +-
 4 files changed, 383 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 7df2d4f..ea06e69 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2606,7 +2606,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..d36b326 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2010-2011 Red Hat, Inc.
- * Copyright (C) 2010 IBM Corporation
+ * Copyright (C) 2010-2012 IBM Corporation
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, 
VIR_NETDEV_MACVLAN_MODE_LAST,
   "passthrough")
 
 #if WITH_MACVTAP
-
 # include 
 # include 
 # include 
@@ -68,6 +67,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_PATTERN  "macvtap%d"
@@ -445,6 +446,328 @@ 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;
+virNetDevVPortProfilePtr virtPortProfile;
+unsigned char *macaddress;
+char *linkdev;
+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
+ * @handled: 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
+virNetDevMacVLanVPortProfileCallback(unsigned char *msg,
+ int length,
+ struct sockaddr_nl *peer,
+ bool *handled,
+ void *opaque)
+{
+   struct nla_policy ifla_vf_policy[IFLA_VF_MAX + 1] = {
+   [IFLA_VF_MAC] = {.minlen = sizeof(struct ifla_vf_mac),
+.maxlen = sizeof(struct ifla_vf_mac)},
+   [IFLA_VF_VLAN] = {.minlen = sizeof(struct ifla_vf_vlan),
+ .maxlen = sizeof(struct ifla_vf_vlan)},
+};
+
+struct nla_policy ifla_port_policy[IFLA_PORT_MAX + 1] = {
+[IFLA_PORT_RESPONSE] = {.type = NLA_U16},
+};
+
+struct nlattr *tb[IFLA_MAX + 1], *tb3[IFLA_PORT_MAX + 1],
+*tb_vfinfo[IFLA_VF_MAX + 1], *tb_vfinfo_list;
+
+struct ifinfomsg ifinfo;
+struct nlmsghdr *hdr;
+void *data;
+int rem;
+char *ifname;
+bool indicate = false;
+virNetlinkCallbackDataPtr calld = opaque;
+pid_t lldpad_pid = 0;
+pid_t virip_pid = 0;
+
+hdr = (struct nlmsghdr *) msg;
+data = nlmsg_data(hdr);
+
+

[libvirt] [PATCHv7 1/2] util: Add netlink event handling to virnetlink.c

2012-02-29 Thread Laine Stump
From: "D. Herrendoerfer" 

This code adds a netlink event interface to libvirt.
It is based upon the event_poll code and makes use of
it. An event is generated for each netlink message sent
to the libvirt pid.

Signed-off-by: D. Herrendoerfer 
---
 daemon/libvirtd.c|8 +
 src/libvirt_private.syms |6 +
 src/util/virnetlink.c|  464 +-
 src/util/virnetlink.h|   33 
 4 files changed, 506 insertions(+), 5 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index b1b542b..ca8074d 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -47,6 +47,7 @@
 #include "conf.h"
 #include "memory.h"
 #include "conf.h"
+#include "virnetlink.h"
 #include "virnetserver.h"
 #include "threads.h"
 #include "remote.h"
@@ -1598,6 +1599,12 @@ int main(int argc, char **argv) {
 goto cleanup;
 }
 
+/* Register the netlink event service */
+if (virNetlinkEventServiceStart() < 0) {
+ret = VIR_DAEMON_ERR_NETWORK;
+goto cleanup;
+}
+
 /* Run event loop. */
 virNetServerRun(srv);
 
@@ -1607,6 +1614,7 @@ int main(int argc, char **argv) {
 0, "shutdown", NULL);
 
 cleanup:
+virNetlinkEventServiceStop();
 virNetServerProgramFree(remoteProgram);
 virNetServerProgramFree(qemuProgram);
 virNetServerClose(srv);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b9baf9a..ce4b7f7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1271,6 +1271,12 @@ virNetDevVPortProfileOpTypeToString;
 
 #virnetlink.h
 virNetlinkCommand;
+virNetlinkEventServiceIsRunning;
+virNetlinkEventServiceStop;
+virNetlinkEventServiceStart;
+virNetlinkEventAddClient;
+virNetlinkEventRemoveClient;
+
 
 
 # virnetmessage.h
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index d03d171..fd6f751 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -35,7 +35,10 @@
 #include 
 
 #include "virnetlink.h"
+#include "logging.h"
 #include "memory.h"
+#include "threads.h"
+#include "virmacaddr.h"
 #include "virterror_internal.h"
 
 #define VIR_FROM_THIS VIR_FROM_NET
@@ -46,6 +49,48 @@
 
 #define NETLINK_ACK_TIMEOUT_S  2
 
+#if defined(__linux__) && defined(HAVE_LIBNL)
+/* State for a single netlink event handle */
+struct virNetlinkEventHandle {
+int watch;
+virNetlinkEventHandleCallback handleCB;
+virNetlinkEventRemoveCallback removeCB;
+void *opaque;
+unsigned char macaddr[VIR_MAC_BUFLEN];
+int deleted;
+};
+
+typedef struct _virNetlinkEventSrvPrivate virNetlinkEventSrvPrivate;
+typedef virNetlinkEventSrvPrivate *virNetlinkEventSrvPrivatePtr;
+struct _virNetlinkEventSrvPrivate {
+/*Server*/
+virMutex lock;
+int eventwatch;
+int netlinkfd;
+struct nl_handle *netlinknh;
+/*Events*/
+int handled;
+size_t handlesCount;
+size_t handlesAlloc;
+struct virNetlinkEventHandle *handles;
+};
+
+enum virNetlinkDeleteMode {
+VIR_NETLINK_HANDLE_VALID,
+VIR_NETLINK_HANDLE_DELETED,
+};
+
+/* Unique ID for the next netlink watch to be registered */
+static int nextWatch = 1;
+
+/* Allocate extra slots for virEventPollHandle/virEventPollTimeout
+ records in this multiple */
+# define NETLINK_EVENT_ALLOC_EXTENT 10
+
+static virNetlinkEventSrvPrivatePtr server = NULL;
+
+/* Function definitions */
+
 /**
  * virNetlinkCommand:
  * @nlmsg: pointer to netlink message
@@ -58,7 +103,6 @@
  * Returns 0 on success, -1 on error. In case of error, no response
  * buffer will be returned.
  */
-#if defined(__linux__) && defined(HAVE_LIBNL)
 int virNetlinkCommand(struct nl_msg *nl_msg,
   unsigned char **respbuf, unsigned int *respbuflen,
   int nl_pid)
@@ -89,7 +133,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 virReportSystemError(errno,
  "%s", _("cannot connect to netlink socket"));
 rc = -1;
-goto err_exit;
+goto error;
 }
 
 nlmsg_set_dst(nl_msg, &nladdr);
@@ -101,7 +145,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 virReportSystemError(errno,
  "%s", _("cannot send to netlink socket"));
 rc = -1;
-goto err_exit;
+goto error;
 }
 
 fd = nl_socket_get_fd(nlhandle);
@@ -118,7 +162,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 virReportSystemError(ETIMEDOUT, "%s",
  _("no valid netlink response was received"));
 rc = -1;
-goto err_exit;
+goto error;
 }
 
 *respbuflen = nl_recv(nlhandle, &nladdr, respbuf, NULL);
@@ -127,7 +171,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
  "%s", _("nl_recv failed"));
 rc = -1;
 }
-err_exit:
+error:
 if (rc == -1) {
 VIR_FREE(*respbuf);
 *respbuf = NULL;
@@ -138,6 +182,345 @@ err_exit:
 return rc;
 }
 
+static void
+virNetlinkEventServerLock(virNetlinkEve

[libvirt] [PATCHv7 0/2] util: Add netlink event handling code

2012-02-29 Thread Laine Stump
This is hopefully the final spin of these patches, It squashes in all
the changes from:

  https://www.redhat.com/archives/libvir-list/2012-February/msg01103.html

as well as:

  https://www.redhat.com/archives/libvir-list/2012-February/msg01173.html

Dirk, please test these out and let me know if they're okay to push.

Thanks!

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


[libvirt] [PATCH] Changes squashed into netlink event service patches

2012-02-29 Thread Laine Stump
Squashed into "util: Add netlink event handling to virnetlink.c" (Note
that I am also re-sending the complete patchset with these changes
already squashed in).

* The remove callback is now an argument of
  virNetlinnkEventAddClient instead of virNetlinkeRemoveClient,
  and is stored in the virNetlinkEventHandle. This way the event
  service shutdown can properly remove any remaining clients to
  avoid memory leaks.

* Several small whitespace changes (moving the opening { of
  functions down to a separate line, for example).

* As said above, the service stop function now removes any
  remaining clients on the list.

* The libvirt convention to check for a failure return from a
  function is (ret < 0) rather than (ret == -1). That was changed
  in two places.

* If macaddr isn't given when adding a client, it is initialized
  to all 0's (on a clean init, it would be 0 anyway, but if the
  entry is being re-used, it would have stale data from the
  previous use).

* In virNetlinkEventRemoveClient, the to if() clauses inside the
  for loop had identical bodies (except for one word in the debug
  message). These have been combined into a single if() clause.

* Again in virNetlinkEventRemoveClient, the opaque object was
  being freed by the event service code, which is supposed to
  treat opaque as a complete unknown. This is no longer done - if
  opaque is dynamically allocated data that needs to be freed,
  this should be done in the client-supplied removal callback (as
  noted earlier, the client has been appropriately changed).

Changes squashed into "Add de-association handling to macvlan code"

* In the virNetCallbackData struct, the pointers to data that are
  in the domain's config object are a bit dangerous - it's
  possible that the domain's config could be updated while the
  domain remains active, freeing the original config object and
  allocating a new one. This would leave the
  virNetlinkCallbackData pointing at garbage data. To fix this,
  the struct has had all consts removed, the object is
  initialized by allocating new memory for each of these items
  and copying (rather than just pointing at the original), and
  the remove callback frees all of the items. The result is that
  the callback data object is completely decoupled from the config
  object.

* Since the callback data object needs to be freed both in the
  remove callback as well as in the error path at the time the
  client is added, a separate virNetlinkCallbackDataFree()
  function has been added. The remove callback now simply calls
  that function.

* Previously the callback data object itself was freed by the
  netlink event service code as the client was being removed. This
  makes it impossible to use the supposedly "opaque" data as
  anything other than a pure pointer to dynamically allocated
  data (a client might want to use it simply as a cookie, or
  pointer to a piece of static data). Now this object is freed by
  the client's remove callback function instead.

* The error path of virNetDevMacVLanCreateWithVPortProfile had
  been missing a call to free the callback data object if the
  strdup(cr_ifname) failed. Now with even more error cases (since
  all the other fields are also dynamically allocated), it became
  even more apparent that disassociate_exit: needed to do
  this. Since all of these cases need to call
  virReportOOMError(), I factored that out into a new goto
  label "memory_error".

* Due to a change in the netlink event service code, the remove callback
  function is now given when calling virNetlinkEventAddClient
  rather than when calling virNetlinkEventRemoveClient.
---
 src/util/virnetdevmacvlan.c |   79 +++--
 src/util/virnetlink.c   |  197 ---
 src/util/virnetlink.h   |7 +-
 3 files changed, 165 insertions(+), 118 deletions(-)

diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 1c4d082..d36b326 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -450,9 +450,9 @@ static const uint32_t modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] 
= {
 struct virNetlinkCallbackData {
 char *cr_ifname;
 virNetDevVPortProfilePtr virtPortProfile;
-const unsigned char *macaddress;
-const char *linkdev;
-const unsigned char *vmuuid;
+unsigned char *macaddress;
+char *linkdev;
+unsigned char *vmuuid;
 enum virNetDevVPortProfileOp vmOp;
 unsigned int linkState;
 };
@@ -726,6 +726,29 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg,
 }
 
 /**
+ * virNetlinkCallbackDataFree
+ *
+ * @calld: pointer to a virNetlinkCallbackData object to free
+ *
+ * This function frees all the data associated with a virNetlinkCallbackData 
object
+ * as well as the object itself. If called with NULL, it does nothing.
+ *
+ * Returns nothing.
+ */
+static void
+virNetlinkCallbackDataFree(virNetlinkCallbackDataPtr calld)
+{
+if (calld) {
+VIR_FREE(calld->cr_ifname);
+   

Re: [libvirt] [PATCH 1/1] complete netlink event integration

2012-02-29 Thread Laine Stump
Since I found a couple other problems, but have made you suffer through
enough back and forth already, I've made some final suggested changes
myself, and am sending a diff patch as a response to this message
(differences from your latest version to mine), as well as doing a
repost of the original two patches with your latest changes and mine
squashed in.

Please 1) take a look at the changes in my diff patches , then 2) test
the updated versions of the full patches (I'm unable to test beyond
compiling), and send your ACK if they're okay. Once I have your ACK
back, I'll push (I promise, I won't find any new issues *this* time :-)

On 02/28/2012 10:34 AM, D. Herrendoerfer wrote:
> From: "D. Herrendoerfer" 
>
> this patch adds the changes proposed by Laine Stump to
> netlink event code.
>
> Signed-off-by: D. Herrendoerfer 
> ---
>  src/util/virnetdevmacvlan.c |   47 +-
>  src/util/virnetlink.c   |   13 ++-
>  src/util/virnetlink.h   |7 -
>  3 files changed, 53 insertions(+), 14 deletions(-)
>
> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
> index 4256349..155ce79 100644
> --- a/src/util/virnetdevmacvlan.c
> +++ b/src/util/virnetdevmacvlan.c
> @@ -448,7 +448,7 @@ static const uint32_t 
> modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = {
>  
>  /* Struct to hold the state and configuration of a 802.1qbg port */
>  struct virNetlinkCallbackData {
> -char cr_ifname[64];
> +char *cr_ifname;
>  virNetDevVPortProfilePtr virtPortProfile;
>  const unsigned char *macaddress;
>  const char *linkdev;
> @@ -606,15 +606,13 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg,
>  if (memcmp(calld->macaddress, m, VIR_MAC_BUFLEN))
>  {
>  /* Repeat the same check for a broadcast mac */
> -unsigned char broadcastmac[VIR_MAC_BUFLEN];
>  int i;
>  
> -for (i = 0;i < VIR_MAC_BUFLEN; i++)
> -broadcastmac[i] = 0xFF;
> -
> -if (memcmp(calld->macaddress, broadcastmac, VIR_MAC_BUFLEN)) 
> {
> -VIR_DEBUG("MAC address match failed.");
> -return;
> +for (i = 0;i < VIR_MAC_BUFLEN; i++) {
> +if (calld->macaddress[i] != 0xff) {
> +VIR_DEBUG("MAC address match failed (wasn't 
> broadcast)");
> +return;
> +}
>  }
>  }
>  }
> @@ -728,6 +726,30 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg,
>  }
>  
>  /**
> + * virNetDevMacVLanVPortProfileDestroyCallback:
> + *
> + * @watch: watch whose handle to remove
> + * @macaddr: macaddr whose handle to remove
> + * @opaque: Contains vital information regarding the associated vm
> + *
> + * This function is called when a netlink message handler is terminated.
> + * The function frees locally allocated data referenced in the opaque
> + * data.
> + */
> +static void
> +virNetDevMacVLanVPortProfileDestroyCallback(int watch ATTRIBUTE_UNUSED,
> +const unsigned char *macaddr 
> ATTRIBUTE_UNUSED,
> +void *opaque)
> +{
> +virNetlinkCallbackDataPtr calld = opaque;
> +
> +if (calld) {
> +VIR_FREE(calld->cr_ifname);
> +}
> +}

You also need to do "VIR_FREE(calld) here, right?

> +
> +
> +/**
>   * virNetDevMacVLanCreateWithVPortProfile:
>   * Create an instance of a macvtap device and open its tap character
>   * device.
> @@ -879,7 +901,12 @@ create_name:
>  goto disassociate_exit;
>  }
>  
> -strncpy(calld->cr_ifname, cr_ifname, 64);
> +calld->cr_ifname=strdup(cr_ifname);
> +if (calld->cr_ifname == NULL) {
> +virReportOOMError();
> +goto disassociate_exit;
> +}
> +
>  calld->virtPortProfile = virtPortProfile;
>  calld->macaddress = macaddress;
>  calld->linkdev = linkdev;
> @@ -938,7 +965,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char 
> *ifname,
>  ret = -1;
>  }
>  
> -virNetlinkEventRemoveClient(0, macaddr);
> +virNetlinkEventRemoveClient(0, macaddr, 
> virNetDevMacVLanVPortProfileDestroyCallback);
>  
>  return ret;
>  }
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index 751aa67..de6a135 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -446,6 +446,7 @@ error:
>   *
>   * @watch: watch whose handle to remove
>   * @macaddr: macaddr whose handle to remove
> + * &cb: callback for the destruction of local data
>   *
>   * Unregister a callback from a netlink monitor.
>   * The handler function referenced will no longer receive netlink messages.
> @@ -454,7 +455,8 @@ error:
>   * returns -1 if the file handle was not registered, 0 upon success
>   */
>  int
> -virNetlinkEventRemoveClient(int watch, const unsigned 

Re: [libvirt] [PATCH] docs: comments wiping supported algorithms

2012-02-29 Thread Daniel Veillard
On Wed, Feb 29, 2012 at 08:24:43AM +, Daniel P. Berrange wrote:
> On Wed, Feb 29, 2012 at 04:02:59PM +0800, Alex Jia wrote:
> > The current scrub version doesn't support pfitzner7, pfitzner33 and 
> > schneier patterns
> > on RHEL, we should comment it in virsh man page.
> > 
> > * tools/virsh.pod: update wiping algorithms docs.
> > 
> > Signed-off-by: Alex Jia 
> > ---
> >  tools/virsh.pod |2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/tools/virsh.pod b/tools/virsh.pod
> > index c306a38..eba9389 100644
> > --- a/tools/virsh.pod
> > +++ b/tools/virsh.pod
> > @@ -2081,6 +2081,8 @@ B
> >pfitzner33 - Roy Pfitzner's 33-random-pass method: random x33.
> >random - 1-pass pattern: random.
> >  
> > +B: Not all algorithms of wiping are expected to work, it depends on 
> > concrete B version. 
> 
> I'd word this slightly differently:
> 
>   B the availability of algorithms may be limited by the version
>   of the C binary installed on the host.

  yeah, that's really more precise :-)

Daniel

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

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


Re: [libvirt] [PATCH] docs: comments wiping supported algorithms

2012-02-29 Thread Daniel P. Berrange
On Wed, Feb 29, 2012 at 04:02:59PM +0800, Alex Jia wrote:
> The current scrub version doesn't support pfitzner7, pfitzner33 and schneier 
> patterns
> on RHEL, we should comment it in virsh man page.
> 
> * tools/virsh.pod: update wiping algorithms docs.
> 
> Signed-off-by: Alex Jia 
> ---
>  tools/virsh.pod |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index c306a38..eba9389 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -2081,6 +2081,8 @@ B
>pfitzner33 - Roy Pfitzner's 33-random-pass method: random x33.
>random - 1-pass pattern: random.
>  
> +B: Not all algorithms of wiping are expected to work, it depends on 
> concrete B version. 

I'd word this slightly differently:

  B the availability of algorithms may be limited by the version
  of the C binary installed on the host.

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] storage: fix a typo

2012-02-29 Thread Peter Krempa

Dňa 29.2.2012 7:21, Alex Jia  wrote / napísal(a):

* src/storage/storage_driver.c (storageVolumeWipeInternal): s/shneier/schneier.

http://code.google.com/p/diskscrub/

Signed-off-by: Alex Jia
---
  src/storage/storage_driver.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)


The man page states the same; ACK

Peter

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

Re: [libvirt] [PATCH] util: fix a typo

2012-02-29 Thread Daniel P. Berrange
On Wed, Feb 29, 2012 at 02:54:38PM +0800, Alex Jia wrote:
> * src/util/event_poll.c: (virEventPollRunOnce): s/imeout/timeout/.
> 
> Signed-off-by: Alex Jia 
> ---
>  src/util/event_poll.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/util/event_poll.c b/src/util/event_poll.c
> index 30dec74..038e75f 100644
> --- a/src/util/event_poll.c
> +++ b/src/util/event_poll.c
> @@ -615,7 +615,7 @@ int virEventPollRunOnce(void) {
>  
>   retry:
>  PROBE(EVENT_POLL_RUN,
> -  "nhandles=%d imeout=%d",
> +  "nhandles=%d timeout=%d",
>nfds, timeout);
>  ret = poll(fds, nfds, timeout);
>  if (ret < 0) {


ACK


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] storage: fix a typo

2012-02-29 Thread Daniel P. Berrange
On Wed, Feb 29, 2012 at 02:21:41PM +0800, Alex Jia wrote:
> * src/storage/storage_driver.c (storageVolumeWipeInternal): 
> s/shneier/schneier.
> 
> http://code.google.com/p/diskscrub/
> 
> Signed-off-by: Alex Jia 
> ---
>  src/storage/storage_driver.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

ACK

> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index df0e291..540e5d7 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -1931,7 +1931,7 @@ storageVolumeWipeInternal(virStorageVolDefPtr def,
>  alg_char = "gutmann";
>  break;
>  case VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER:
> -alg_char = "shneier";
> +alg_char = "schneier";
>  break;
>  case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7:
>  alg_char = "pfitzner7";
> -- 


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] Need your help with virsh desc

2012-02-29 Thread Daniel Veillard
On Wed, Feb 29, 2012 at 03:08:30AM -0500, Zhimou Peng wrote:
> Hi Eric,
> I discussed this in my team.
> And i don't think refuse newlines is a good idea. Description can contains 
> several lines is better.
> I agree with you of the second one, keep the format of user's input is 
> better(virt-manager shows description too).
> And i need more information from upstream libvirt list, pls help.

  I completely agree with Eric here, the description is user data
and we cannot modify it. Now what you want is a short description
i.e. a title and that was added in 0.9.10

  
http://libvirt.org/git/?p=libvirt.git;a=commit;h=b79ba8382e2205c416d7c4836ac9ee08c72e2c56

  use that construct instead but don't ask us to modify description
because it is in use already for complex use case and I will definitely
not modify that data on input, it would be a terrible regression.

Daniel

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

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


Re: [libvirt] [PATCH] docs: comments wiping supported algorithms

2012-02-29 Thread Daniel Veillard
On Wed, Feb 29, 2012 at 04:13:52PM +0800, Alex Jia wrote:
> On 02/29/2012 04:02 PM, Alex Jia wrote:
> >The current scrub version doesn't support pfitzner7, pfitzner33 and schneier 
> >patterns
> >on RHEL, we should comment it in virsh man page.
> >
> >* tools/virsh.pod: update wiping algorithms docs.
> >
> >Signed-off-by: Alex Jia
> >---
> >  tools/virsh.pod |2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> >diff --git a/tools/virsh.pod b/tools/virsh.pod
> >index c306a38..eba9389 100644
> >--- a/tools/virsh.pod
> >+++ b/tools/virsh.pod
> >@@ -2081,6 +2081,8 @@ B
> >pfitzner33 - Roy Pfitzner's 33-random-pass method: random x33.
> >random - 1-pass pattern: random.
> >
> >+B: Not all algorithms of wiping are expected to work, it depends on 
> >concrete B  version.
> s/concrete/specific/, it should make more sense.

  ACK to that :-)

Daniel

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

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


Re: [libvirt] Need your help with virsh desc

2012-02-29 Thread Zhimou Peng
Hi Eric,
I discussed this in my team.
And i don't think refuse newlines is a good idea. Description can contains 
several lines is better.
I agree with you of the second one, keep the format of user's input is 
better(virt-manager shows description too).
And i need more information from upstream libvirt list, pls help.

Thanks.
zhpeng
BR
- Original Message -
From: "Eric Blake" 
To: "Zhimou Peng" 
Cc: "Alex Jia" , "Rita Wu" 
Sent: Wednesday, February 29, 2012 12:28:48 AM
Subject: Re: Need your help with virsh desc

On 02/27/2012 10:49 PM, Zhimou Peng wrote:
> Hi Eric,
> 
>I try to add some description to my guest, and i find xml format isn't 
> like good.
> 
> For example:
> When i add/edit description with:
> 
> # virsh desc kvm1 asdfsadfssafsadfa
> Domain description updated successfully
> # virsh dumpxml kvm1
> 
>   kvm1
>   d5cd1756-eb3b-fd9b-1f91-0cc801dfc622
>   asdfsadfssafsadfa--> looks good
> 
> # virsh desc --edit --config kvm1
> This is a test desc of kvm1
> 
> # virsh dumpxml kvm1
> 
>   kvm1
>   d5cd1756-eb3b-fd9b-1f91-0cc801dfc622
>   This is a test desc of kvm1> I think 
> the end of line *$* should be delete
>  
> 

This is a case where your editor added a trailing newline, and libvirt
passed that newline on through to the xml.

We are already deleting the newline for  (which refuses
newlines), but  allows newlines.  I think we should open a
BZ to have virsh strip the trailing newline always, and not just for
.

> Expect result:
> 
>   kvm1
>   d5cd1756-eb3b-fd9b-1f91-0cc801dfc622
>   This is a test desc of kvm1 
> 
> --
> Description can contain several lines:
> 
> # virsh desc --edit --config kvm2
> This is a test desc of kvm2
> We have 3 lines
> test test test
> 
> # virsh dumpxml kvm2
> 
>   kvm1
>   d5cd1756-eb3b-fd9b-1f91-0cc801dfc622
>   This is a test desc of kvm2
> We have 3 lines
> test test test
> -> I think 
> libvirt should uniform XML format in guest configuration.
> 
> Expect result:
> 
>   kvm1
>   d5cd1756-eb3b-fd9b-1f91-0cc801dfc622
>   
> This is a test desc of kvm2
> We have 3 lines
> test test test
> 

Sorry.  We _cannot_ alter the whitespace of the text in the element.
While whitespace in between <> tags is flexible (and thus we format
things to make it look nested), whitespace within the text surrounded by
a  block is significant, because it was
provided by the user, and must be given back to the user in the same
format.  Perhaps we could encode things using �a; instead of a literal
newline, as in:

  Line 1�a;Line 2

but I don't know if libxml2 can be made to do that reliably.  This
question would be better asked on the upstream libvirt list.

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

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


Re: [libvirt] [PATCH] docs: comments wiping supported algorithms

2012-02-29 Thread Alex Jia

On 02/29/2012 04:02 PM, Alex Jia wrote:

The current scrub version doesn't support pfitzner7, pfitzner33 and schneier 
patterns
on RHEL, we should comment it in virsh man page.

* tools/virsh.pod: update wiping algorithms docs.

Signed-off-by: Alex Jia
---
  tools/virsh.pod |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index c306a38..eba9389 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2081,6 +2081,8 @@ B
pfitzner33 - Roy Pfitzner's 33-random-pass method: random x33.
random - 1-pass pattern: random.

+B: Not all algorithms of wiping are expected to work, it depends on concrete 
B  version.

s/concrete/specific/, it should make more sense.

+
  =item B  [I<--pool>  I] I

  Output the volume information as an XML dump to stdout.


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


[libvirt] [PATCH] docs: comments wiping supported algorithms

2012-02-29 Thread Alex Jia
The current scrub version doesn't support pfitzner7, pfitzner33 and schneier 
patterns
on RHEL, we should comment it in virsh man page.

* tools/virsh.pod: update wiping algorithms docs.

Signed-off-by: Alex Jia 
---
 tools/virsh.pod |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index c306a38..eba9389 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2081,6 +2081,8 @@ B
   pfitzner33 - Roy Pfitzner's 33-random-pass method: random x33.
   random - 1-pass pattern: random.
 
+B: Not all algorithms of wiping are expected to work, it depends on 
concrete B version. 
+
 =item B [I<--pool> I] I
 
 Output the volume information as an XML dump to stdout.
-- 
1.7.1

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