[libvirt] [PATCH] daemon: Plug memory leaks

2012-04-12 Thread Alex Jia
Detected by valgrind. Leaks are introduced in commit 6e6e9be.

* daemon/libvirtd-config.c (macro GET_CONF_STR): fix memory leaks.

How to reproduce?

% make  make -C tests check TESTS=libvirtdconftest
% cd tests  valgrind -v --leak-check=full ./libvirtdconftest

actual result:

==11008== 185 bytes in 5 blocks are definitely lost in loss record 3 of 5
==11008==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==11008==by 0x39CF07F6E1: strdup (strdup.c:43)
==11008==by 0x406626: daemonConfigLoadOptions (libvirtd-config.c:438)
==11008==by 0x406800: daemonConfigLoadData (libvirtd-config.c:492)
==11008==by 0x403CCF: testCorrupt (libvirtdconftest.c:110)
==11008==by 0x404FAD: virtTestRun (testutils.c:145)
==11008==by 0x403A34: mymain (libvirtdconftest.c:219)
==11008==by 0x404687: virtTestMain (testutils.c:700)
==11008==by 0x39CF01ECDC: (below main) (libc-start.c:226)
==11008==
==11008== LEAK SUMMARY:
==11008==definitely lost: 185 bytes in 5 blocks

Signed-off-by: Alex Jia a...@redhat.com
---
 daemon/libvirtd-config.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index 4d041f0..0a4323a 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -152,6 +152,7 @@ checkType (virConfValuePtr p, const char *filename,
 virReportOOMError();\
 goto error; \
 }   \
+VIR_FREE(data-var_name);   \
 }   \
 } while (0)
 
-- 
1.7.1

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


Re: [libvirt] [libvirt-gconfig PATCHv2 14/14] Implement gvir_config_domain_setup_default_usb_controllers

2012-04-12 Thread Christophe Fergeau
On Wed, Apr 11, 2012 at 05:19:33PM +0100, Daniel P. Berrange wrote:
 The concept of 'default usb controllers' seems very policy based
 to me  different hypervisors will have different views of this.

With respect to hypervisors, since the function has a GVirConfigDomain
parameter, we could use it to have different defaults depending on the
hypervisor/arch/...

Christophe


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

Re: [libvirt] [libvirt-gconfig PATCHv2 14/14] Implement gvir_config_domain_setup_default_usb_controllers

2012-04-12 Thread Christophe Fergeau
On Wed, Apr 11, 2012 at 06:51:48PM +0200, Marc-André Lureau wrote:
 On Wed, Apr 11, 2012 at 6:19 PM, Daniel P. Berrange berra...@redhat.com 
 wrote:
  The concept of 'default usb controllers' seems very policy based
  to me  different hypervisors will have different views of this.
 
 This is a helper, it is not imposing any policy, but giving you sane
 default setup for ICH9 USB2 controllers if you want to.

Deciding on what the default setup is is some kind of policy ) But yeah
it's not imposed since there are still the lower level function.

That said, this function doing something magical is quite odd compared to
the rest of libvirt-gconfig API which directly manipulate xml nodes. A
simpler API will probably needed at some point, but maybe it belongs just
above libvirt-gconfig instead of inside it.

Christophe


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

Re: [libvirt] [PATCH] daemon: Plug memory leaks

2012-04-12 Thread Daniel P. Berrange
On Thu, Apr 12, 2012 at 03:24:08PM +0800, Alex Jia wrote:
 Detected by valgrind. Leaks are introduced in commit 6e6e9be.
 
 * daemon/libvirtd-config.c (macro GET_CONF_STR): fix memory leaks.
 
 How to reproduce?
 
 % make  make -C tests check TESTS=libvirtdconftest
 % cd tests  valgrind -v --leak-check=full ./libvirtdconftest
 
 actual result:
 
 ==11008== 185 bytes in 5 blocks are definitely lost in loss record 3 of 5
 ==11008==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
 ==11008==by 0x39CF07F6E1: strdup (strdup.c:43)
 ==11008==by 0x406626: daemonConfigLoadOptions (libvirtd-config.c:438)
 ==11008==by 0x406800: daemonConfigLoadData (libvirtd-config.c:492)
 ==11008==by 0x403CCF: testCorrupt (libvirtdconftest.c:110)
 ==11008==by 0x404FAD: virtTestRun (testutils.c:145)
 ==11008==by 0x403A34: mymain (libvirtdconftest.c:219)
 ==11008==by 0x404687: virtTestMain (testutils.c:700)
 ==11008==by 0x39CF01ECDC: (below main) (libc-start.c:226)
 ==11008==
 ==11008== LEAK SUMMARY:
 ==11008==definitely lost: 185 bytes in 5 blocks
 
 Signed-off-by: Alex Jia a...@redhat.com
 ---
  daemon/libvirtd-config.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
 index 4d041f0..0a4323a 100644
 --- a/daemon/libvirtd-config.c
 +++ b/daemon/libvirtd-config.c
 @@ -152,6 +152,7 @@ checkType (virConfValuePtr p, const char *filename,
  virReportOOMError();\
  goto error; \
  }   \
 +VIR_FREE(data-var_name);   \
  }   \
  } while (0)

Errr, no. Please look at the context of what's going on, rather than
just blindly adding code to free a block at the point where valgrind
shows the allocation.

The full context to this diff

#define GET_CONF_STR(conf, filename, var_name)  \
do {\
virConfValuePtr p = virConfGetValue (conf, #var_name);  \
if (p) {\
if (checkType (p, filename, #var_name, VIR_CONF_STRING)  0) \
goto error; \
VIR_FREE(data-var_name);   \
if (!(data-var_name = strdup (p-str))) {  \
virReportOOMError();\
goto error; \
}   \
}   \
} while (0)


Your suggested change means we would be free'ing the config file before
we have even used it.

If you look at this line:

 ==11008==by 0x406626: daemonConfigLoadOptions (libvirtd-config.c:438)

You'll see it matches:

GET_CONF_STR (conf, filename, host_uuid);

There are no leak reports for other uses of GET_CONF_STR, so this suggests
that 'host_uuid' is not being freed. Check daemonConfigFree and you'll see
this is indeed missing

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] Getter/setter for disk source's startupPolicy attribute

2012-04-12 Thread Christophe Fergeau
Hey,

On Thu, Apr 12, 2012 at 05:54:25AM +0300, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org
 
 ---
  libvirt-gconfig/libvirt-gconfig-domain-disk.c |   26 
 +
  libvirt-gconfig/libvirt-gconfig-domain-disk.h |9 
  libvirt-gconfig/libvirt-gconfig.sym   |3 ++

Can you add a call to _set_startup_policy() to test-domain-create.c + a
g_assert(_get_startup_policy() == ...)? Ack with this added.


Christophe

  3 files changed, 38 insertions(+), 0 deletions(-)
 
 diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.c 
 b/libvirt-gconfig/libvirt-gconfig-domain-disk.c
 index 5d0acb5..a29ea47 100644
 --- a/libvirt-gconfig/libvirt-gconfig-domain-disk.c
 +++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.c
 @@ -127,6 +127,18 @@ void 
 gvir_config_domain_disk_set_snapshot_type(GVirConfigDomainDisk *disk,
 type, NULL);
  }
  
 +void gvir_config_domain_disk_set_startup_policy(GVirConfigDomainDisk *disk,
 +
 GVirConfigDomainDiskStartupPolicy policy)
 +{
 +const char *str;
 +
 +g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk));
 +str = 
 gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_DISK_STARTUP_POLICY, 
 policy);
 +g_return_if_fail(str != NULL);
 +gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk),
 +source, startupPolicy, 
 str);
 +}
 +
  void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk,
  const char *source)
  {
 @@ -235,6 +247,19 @@ 
 gvir_config_domain_disk_get_snapshot_type(GVirConfigDomainDisk *disk)

 GVIR_CONFIG_DOMAIN_DISK_SNAPSHOT_NO);
  }
  
 +GVirConfigDomainDiskStartupPolicy
 +gvir_config_domain_disk_get_startup_policy(GVirConfigDomainDisk *disk)
 +{
 +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk),
 + GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_MANDATORY);
 +
 +return gvir_config_object_get_attribute_genum
 +(GVIR_CONFIG_OBJECT(disk),
 + source, startupPolicy,
 + GVIR_CONFIG_TYPE_DOMAIN_DISK_STARTUP_POLICY,
 + GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_MANDATORY);
 +}
 +
  const char *
  gvir_config_domain_disk_get_source(GVirConfigDomainDisk *disk)
  {
 @@ -291,6 +316,7 @@ 
 gvir_config_domain_disk_get_driver_cache(GVirConfigDomainDisk *disk)

 GVIR_CONFIG_TYPE_DOMAIN_DISK_CACHE_TYPE,

 GVIR_CONFIG_DOMAIN_DISK_CACHE_DEFAULT);
  }
 +
  GVirConfigDomainDiskBus
  gvir_config_domain_disk_get_target_bus(GVirConfigDomainDisk *disk)
  {
 diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.h 
 b/libvirt-gconfig/libvirt-gconfig-domain-disk.h
 index 916421d..7e85d75 100644
 --- a/libvirt-gconfig/libvirt-gconfig-domain-disk.h
 +++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.h
 @@ -95,6 +95,12 @@ typedef enum {
  GVIR_CONFIG_DOMAIN_DISK_SNAPSHOT_EXTERNAL
  } GVirConfigDomainDiskSnapshotType;
  
 +typedef enum {
 +GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_MANDATORY,
 +GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_REQUISITE,
 +GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_OPTIONAL
 +} GVirConfigDomainDiskStartupPolicy;
 +
  GType gvir_config_domain_disk_get_type(void);
  
  GVirConfigDomainDisk *gvir_config_domain_disk_new(void);
 @@ -107,6 +113,8 @@ void 
 gvir_config_domain_disk_set_guest_device_type(GVirConfigDomainDisk *disk,
 
 GVirConfigDomainDiskGuestDeviceType type);
  void gvir_config_domain_disk_set_snapshot_type(GVirConfigDomainDisk *disk,
 
 GVirConfigDomainDiskSnapshotType type);
 +void gvir_config_domain_disk_set_startup_policy(GVirConfigDomainDisk *disk,
 +
 GVirConfigDomainDiskStartupPolicy policy);
  void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk,
  const char *source);
  void gvir_config_domain_disk_set_driver_cache(GVirConfigDomainDisk *disk,
 @@ -123,6 +131,7 @@ void 
 gvir_config_domain_disk_set_target_dev(GVirConfigDomainDisk *disk,
  GVirConfigDomainDiskType 
 gvir_config_domain_disk_get_disk_type(GVirConfigDomainDisk *disk);
  GVirConfigDomainDiskGuestDeviceType 
 gvir_config_domain_disk_get_guest_device_type(GVirConfigDomainDisk *disk);
  GVirConfigDomainDiskSnapshotType 
 gvir_config_domain_disk_get_snapshot_type(GVirConfigDomainDisk *disk);
 +GVirConfigDomainDiskStartupPolicy 
 gvir_config_domain_disk_get_startup_policy(GVirConfigDomainDisk *disk);
  const char *gvir_config_domain_disk_get_source(GVirConfigDomainDisk *disk);
  GVirConfigDomainDiskCacheType 
 

Re: [libvirt] [PATCH] daemon: Plug memory leaks

2012-04-12 Thread Alex Jia

On 04/12/2012 04:37 PM, Daniel P. Berrange wrote:

On Thu, Apr 12, 2012 at 03:24:08PM +0800, Alex Jia wrote:

Detected by valgrind. Leaks are introduced in commit 6e6e9be.

* daemon/libvirtd-config.c (macro GET_CONF_STR): fix memory leaks.

How to reproduce?

% make  make -C tests check TESTS=libvirtdconftest
% cd tests  valgrind -v --leak-check=full ./libvirtdconftest

actual result:

==11008== 185 bytes in 5 blocks are definitely lost in loss record 3 of 5
==11008==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==11008==by 0x39CF07F6E1: strdup (strdup.c:43)
==11008==by 0x406626: daemonConfigLoadOptions (libvirtd-config.c:438)
==11008==by 0x406800: daemonConfigLoadData (libvirtd-config.c:492)
==11008==by 0x403CCF: testCorrupt (libvirtdconftest.c:110)
==11008==by 0x404FAD: virtTestRun (testutils.c:145)
==11008==by 0x403A34: mymain (libvirtdconftest.c:219)
==11008==by 0x404687: virtTestMain (testutils.c:700)
==11008==by 0x39CF01ECDC: (below main) (libc-start.c:226)
==11008==
==11008== LEAK SUMMARY:
==11008==definitely lost: 185 bytes in 5 blocks

Signed-off-by: Alex Jiaa...@redhat.com
---
  daemon/libvirtd-config.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index 4d041f0..0a4323a 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -152,6 +152,7 @@ checkType (virConfValuePtr p, const char *filename,
  virReportOOMError();\
  goto error; \
  }   \
+VIR_FREE(data-var_name);   \
  }   \
  } while (0)

Errr, no. Please look at the context of what's going on, rather than
just blindly adding code to free a block at the point where valgrind
shows the allocation.

The full context to this diff

#define GET_CONF_STR(conf, filename, var_name)  \
 do {\
 virConfValuePtr p = virConfGetValue (conf, #var_name);  \
 if (p) {\
 if (checkType (p, filename, #var_name, VIR_CONF_STRING)  0) \
 goto error; \
 VIR_FREE(data-var_name);   \
 if (!(data-var_name = strdup (p-str))) {  \
 virReportOOMError();\
 goto error; \
 }   \
 }   \
 } while (0)


Your suggested change means we would be free'ing the config file before
we have even used it.

If you look at this line:

Yeah.

==11008==by 0x406626: daemonConfigLoadOptions (libvirtd-config.c:438)

You'll see it matches:

 GET_CONF_STR (conf, filename, host_uuid);

There are no leak reports for other uses of GET_CONF_STR, so this suggests
that 'host_uuid' is not being freed. Check daemonConfigFree and you'll see
this is indeed missing
Yeah, it's a little weird, the previous codes also haven't free 
'host_uuid', but valgrind hasn't complains memory leak.


Thanks for your review,
Alex

Daniel


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


[libvirt] [PATCHv2] daemon: Plug memory leaks

2012-04-12 Thread Alex Jia
* daemon/libvirtd-config.c (daemonConfigFree): fix memory leaks.

How to reproduce?

% make  make -C tests check TESTS=libvirtdconftest
% cd tests  valgrind -v --leak-check=full ./libvirtdconftest

actual result:

==11008== 185 bytes in 5 blocks are definitely lost in loss record 3 of 5
==11008==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==11008==by 0x39CF07F6E1: strdup (strdup.c:43)
==11008==by 0x406626: daemonConfigLoadOptions (libvirtd-config.c:438)
==11008==by 0x406800: daemonConfigLoadData (libvirtd-config.c:492)
==11008==by 0x403CCF: testCorrupt (libvirtdconftest.c:110)
==11008==by 0x404FAD: virtTestRun (testutils.c:145)
==11008==by 0x403A34: mymain (libvirtdconftest.c:219)
==11008==by 0x404687: virtTestMain (testutils.c:700)
==11008==by 0x39CF01ECDC: (below main) (libc-start.c:226)
==11008==
==11008== LEAK SUMMARY:
==11008==definitely lost: 185 bytes in 5 blocks

Signed-off-by: Alex Jia a...@redhat.com
---
 daemon/libvirtd-config.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index 4d041f0..471236c 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -358,6 +358,7 @@ daemonConfigFree(struct daemonConfig *data)
 VIR_FREE(data-cert_file);
 VIR_FREE(data-crl_file);
 
+VIR_FREE(data-host_uuid);
 VIR_FREE(data-log_filters);
 VIR_FREE(data-log_outputs);
 
-- 
1.7.1

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


Re: [libvirt] [PATCHv2] daemon: Plug memory leaks

2012-04-12 Thread Daniel P. Berrange
On Thu, Apr 12, 2012 at 05:12:12PM +0800, Alex Jia wrote:
 * daemon/libvirtd-config.c (daemonConfigFree): fix memory leaks.
 
 How to reproduce?
 
 % make  make -C tests check TESTS=libvirtdconftest
 % cd tests  valgrind -v --leak-check=full ./libvirtdconftest
 
 actual result:
 
 ==11008== 185 bytes in 5 blocks are definitely lost in loss record 3 of 5
 ==11008==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
 ==11008==by 0x39CF07F6E1: strdup (strdup.c:43)
 ==11008==by 0x406626: daemonConfigLoadOptions (libvirtd-config.c:438)
 ==11008==by 0x406800: daemonConfigLoadData (libvirtd-config.c:492)
 ==11008==by 0x403CCF: testCorrupt (libvirtdconftest.c:110)
 ==11008==by 0x404FAD: virtTestRun (testutils.c:145)
 ==11008==by 0x403A34: mymain (libvirtdconftest.c:219)
 ==11008==by 0x404687: virtTestMain (testutils.c:700)
 ==11008==by 0x39CF01ECDC: (below main) (libc-start.c:226)
 ==11008==
 ==11008== LEAK SUMMARY:
 ==11008==definitely lost: 185 bytes in 5 blocks
 
 Signed-off-by: Alex Jia a...@redhat.com
 ---
  daemon/libvirtd-config.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
 index 4d041f0..471236c 100644
 --- a/daemon/libvirtd-config.c
 +++ b/daemon/libvirtd-config.c
 @@ -358,6 +358,7 @@ daemonConfigFree(struct daemonConfig *data)
  VIR_FREE(data-cert_file);
  VIR_FREE(data-crl_file);
  
 +VIR_FREE(data-host_uuid);
  VIR_FREE(data-log_filters);
  VIR_FREE(data-log_outputs);

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] [PATCHv2] daemon: Plug memory leaks

2012-04-12 Thread Alex Jia

On 04/12/2012 05:12 PM, Daniel P. Berrange wrote:

On Thu, Apr 12, 2012 at 05:12:12PM +0800, Alex Jia wrote:

* daemon/libvirtd-config.c (daemonConfigFree): fix memory leaks.

How to reproduce?

% make  make -C tests check TESTS=libvirtdconftest
% cd tests  valgrind -v --leak-check=full ./libvirtdconftest

actual result:

==11008== 185 bytes in 5 blocks are definitely lost in loss record 3 of 5
==11008==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==11008==by 0x39CF07F6E1: strdup (strdup.c:43)
==11008==by 0x406626: daemonConfigLoadOptions (libvirtd-config.c:438)
==11008==by 0x406800: daemonConfigLoadData (libvirtd-config.c:492)
==11008==by 0x403CCF: testCorrupt (libvirtdconftest.c:110)
==11008==by 0x404FAD: virtTestRun (testutils.c:145)
==11008==by 0x403A34: mymain (libvirtdconftest.c:219)
==11008==by 0x404687: virtTestMain (testutils.c:700)
==11008==by 0x39CF01ECDC: (below main) (libc-start.c:226)
==11008==
==11008== LEAK SUMMARY:
==11008==definitely lost: 185 bytes in 5 blocks

Signed-off-by: Alex Jiaa...@redhat.com
---
  daemon/libvirtd-config.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index 4d041f0..471236c 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -358,6 +358,7 @@ daemonConfigFree(struct daemonConfig *data)
  VIR_FREE(data-cert_file);
  VIR_FREE(data-crl_file);

+VIR_FREE(data-host_uuid);
  VIR_FREE(data-log_filters);
  VIR_FREE(data-log_outputs);

ACK

Daniel

Thanks and pushed now.

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


Re: [libvirt] [libvirt-gconfig PATCHv2 01/14] Add GVirConfigDomainController skeleton

2012-04-12 Thread Guido Günther
On Wed, Apr 11, 2012 at 03:48:09PM +0200, Christophe Fergeau wrote:
[..snip..] 
 --- a/libvirt-gconfig/libvirt-gconfig.sym
 +++ b/libvirt-gconfig/libvirt-gconfig.sym
 @@ -69,6 +69,8 @@ LIBVIRT_GCONFIG_0.0.4 {
   gvir_config_domain_console_set_target_type;
   gvir_config_domain_console_target_type_get_type;
  
 + gvir_config_domain_controller_get_type;
 +
   gvir_config_domain_device_get_type;
  
   gvir_config_domain_disk_get_type;
 -- 
Wouldn't this need to go into LIBVIRT_GCONFIG_0.0.8 by either
introducing a new section or bumping LIBVIRT_GCONFIG_0.0.X in general?
Cheers,
 -- Guido

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


Re: [libvirt] [libvirt-glib] Getter/setter for disk source's startupPolicy attribute

2012-04-12 Thread Guido Günther
On Thu, Apr 12, 2012 at 05:54:25AM +0300, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org
 
 ---
  libvirt-gconfig/libvirt-gconfig-domain-disk.c |   26 
 +
  libvirt-gconfig/libvirt-gconfig-domain-disk.h |9 
  libvirt-gconfig/libvirt-gconfig.sym   |3 ++
  3 files changed, 38 insertions(+), 0 deletions(-)
 
 diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.c 
 b/libvirt-gconfig/libvirt-gconfig-domain-disk.c
 index 5d0acb5..a29ea47 100644
 --- a/libvirt-gconfig/libvirt-gconfig-domain-disk.c
 +++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.c
 @@ -127,6 +127,18 @@ void 
 gvir_config_domain_disk_set_snapshot_type(GVirConfigDomainDisk *disk,
 type, NULL);
  }
  
 +void gvir_config_domain_disk_set_startup_policy(GVirConfigDomainDisk *disk,
 +
 GVirConfigDomainDiskStartupPolicy policy)
 +{
 +const char *str;
 +
 +g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk));
 +str = 
 gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_DISK_STARTUP_POLICY, 
 policy);
 +g_return_if_fail(str != NULL);
 +gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk),
 +source, startupPolicy, 
 str);
 +}
 +
  void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk,
  const char *source)
  {
 @@ -235,6 +247,19 @@ 
 gvir_config_domain_disk_get_snapshot_type(GVirConfigDomainDisk *disk)

 GVIR_CONFIG_DOMAIN_DISK_SNAPSHOT_NO);
  }
  
 +GVirConfigDomainDiskStartupPolicy
 +gvir_config_domain_disk_get_startup_policy(GVirConfigDomainDisk *disk)
 +{
 +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk),
 + GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_MANDATORY);
 +
 +return gvir_config_object_get_attribute_genum
 +(GVIR_CONFIG_OBJECT(disk),
 + source, startupPolicy,
 + GVIR_CONFIG_TYPE_DOMAIN_DISK_STARTUP_POLICY,
 + GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_MANDATORY);
 +}
 +
  const char *
  gvir_config_domain_disk_get_source(GVirConfigDomainDisk *disk)
  {
 @@ -291,6 +316,7 @@ 
 gvir_config_domain_disk_get_driver_cache(GVirConfigDomainDisk *disk)

 GVIR_CONFIG_TYPE_DOMAIN_DISK_CACHE_TYPE,

 GVIR_CONFIG_DOMAIN_DISK_CACHE_DEFAULT);
  }
 +
  GVirConfigDomainDiskBus
  gvir_config_domain_disk_get_target_bus(GVirConfigDomainDisk *disk)
  {
 diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.h 
 b/libvirt-gconfig/libvirt-gconfig-domain-disk.h
 index 916421d..7e85d75 100644
 --- a/libvirt-gconfig/libvirt-gconfig-domain-disk.h
 +++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.h
 @@ -95,6 +95,12 @@ typedef enum {
  GVIR_CONFIG_DOMAIN_DISK_SNAPSHOT_EXTERNAL
  } GVirConfigDomainDiskSnapshotType;
  
 +typedef enum {
 +GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_MANDATORY,
 +GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_REQUISITE,
 +GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_OPTIONAL
 +} GVirConfigDomainDiskStartupPolicy;
 +
  GType gvir_config_domain_disk_get_type(void);
  
  GVirConfigDomainDisk *gvir_config_domain_disk_new(void);
 @@ -107,6 +113,8 @@ void 
 gvir_config_domain_disk_set_guest_device_type(GVirConfigDomainDisk *disk,
 
 GVirConfigDomainDiskGuestDeviceType type);
  void gvir_config_domain_disk_set_snapshot_type(GVirConfigDomainDisk *disk,
 
 GVirConfigDomainDiskSnapshotType type);
 +void gvir_config_domain_disk_set_startup_policy(GVirConfigDomainDisk *disk,
 +
 GVirConfigDomainDiskStartupPolicy policy);
  void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk,
  const char *source);
  void gvir_config_domain_disk_set_driver_cache(GVirConfigDomainDisk *disk,
 @@ -123,6 +131,7 @@ void 
 gvir_config_domain_disk_set_target_dev(GVirConfigDomainDisk *disk,
  GVirConfigDomainDiskType 
 gvir_config_domain_disk_get_disk_type(GVirConfigDomainDisk *disk);
  GVirConfigDomainDiskGuestDeviceType 
 gvir_config_domain_disk_get_guest_device_type(GVirConfigDomainDisk *disk);
  GVirConfigDomainDiskSnapshotType 
 gvir_config_domain_disk_get_snapshot_type(GVirConfigDomainDisk *disk);
 +GVirConfigDomainDiskStartupPolicy 
 gvir_config_domain_disk_get_startup_policy(GVirConfigDomainDisk *disk);
  const char *gvir_config_domain_disk_get_source(GVirConfigDomainDisk *disk);
  GVirConfigDomainDiskCacheType 
 gvir_config_domain_disk_get_driver_cache(GVirConfigDomainDisk *disk);
  const char *gvir_config_domain_disk_get_driver_name(GVirConfigDomainDisk 
 *disk);
 diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
 

Re: [libvirt] [libvirt-gconfig PATCHv2 01/14] Add GVirConfigDomainController skeleton

2012-04-12 Thread Christophe Fergeau
On Thu, Apr 12, 2012 at 12:06:52PM +0200, Guido Günther wrote:
 On Wed, Apr 11, 2012 at 03:48:09PM +0200, Christophe Fergeau wrote:
 [..snip..] 
  --- a/libvirt-gconfig/libvirt-gconfig.sym
  +++ b/libvirt-gconfig/libvirt-gconfig.sym
  @@ -69,6 +69,8 @@ LIBVIRT_GCONFIG_0.0.4 {
  gvir_config_domain_console_set_target_type;
  gvir_config_domain_console_target_type_get_type;
   
  +   gvir_config_domain_controller_get_type;
  +
  gvir_config_domain_device_get_type;
   
  gvir_config_domain_disk_get_type;
  -- 
 Wouldn't this need to go into LIBVIRT_GCONFIG_0.0.8 by either
 introducing a new section or bumping LIBVIRT_GCONFIG_0.0.X in general?

Yep, we generally do this once before releasing (and forget :-/), I'll add
this to the first patch adding new symbols.

Christophe


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

Re: [libvirt] [PATCH] qemuOpenFile: Don't force chown on NFS

2012-04-12 Thread Laine Stump
On 04/11/2012 04:21 PM, Laine Stump wrote:
 ACK to the idea, but NACK to the exact placement of the fix.


On further examination (and actually doing a couple tests), I withdraw
my NACK on the placement. I had mixed up usage of qemuOpenFile and
virFileOpen in my memory - qemuOpenFile already hardly ever cares about
the ownership of a file it opens, and even when it does, it always wants
it to be owned by root (well, actually getuid()).

So, although putting the fix in the place I suggested does work, my
reasoning was flawed and your original position also works properly, as
well as keeping the logic for setting the FORCE_OWNER flag in one place
- ACK.

Sorry for the noise :-)


 On 04/11/2012 05:17 AM, Michal Privoznik wrote:
 If dynamic_ownership is off and we are creating a file on NFS
 we force chown. This will fail as chown/chmod are not supported
 on NFS. However, with no dynamic_ownership we are not required
 to do any chown.
 This statement isn't exactly correct - chown/chmod *are* supported on
 NFS, but won't work if the NFS share is mounted with root-squash,
 because only root can chown and all commands issued by root appear to
 the server as being from user nobody.

 The fix also isn't correct. If dynamic_ownership is off, that means that
 we don't want the DAC security driver to dynamically chown files back
 and forth between uid 0 and uid qemu_user. However, it's very possible
 that we would want to create a new file that is permanently owned by uid
 qemu_user, and one way to do that is to create the file, then chown it
 (the other way, which is only used if root is unable to create the file,
 is to fork, setuid to the qemu_user, then open the file). (NB: here it
 is the mainline code that is chowning the file, *not* the DAC security
 driver, and it is a one-time change, not something dynamic that will
 later be reverted).

 If we want a file created that is owned by the qemu user, and that file
 is located on a *non*-root-squash NFS, the proper (only?) way to do this
 is to set the FORCE_OWNER flag when calling virFileOpenAs(). Its first
 attempt will be to open the file then (only if FORCE_OWNER is set!)
 chown it.

 With this fix in place, we are resetting the FORCE_OWNER flag from the
 very beginning, so the file will be created owned by uid 0, and chown
 will never be called, leaving us with a file that is inaccessible to a
 qemu process running with uid qemu_user. (fortunately, in the case of
 doing a domain save, that isn't a problem, since it's libvirt that needs
 to open the file, and it only passes an fd to qemu, but the result is
 still incorrect, and there may be other uses of qemuOpenFile() that
 aren't as forgiving).

 Instead, I think what should be done is to allow the first attempt at
 calling virFileOpenAs() without resetting the FORCE_OWNER flag. If it is
 successful, then we have a new file owned by qemu_user, and we're done.
 If it fails, *then* reset the FORCE_OWNER flag just before the 2nd call
 to virFileOpenAs (iff dynamic_ownership is false).

 I'm starting a build with an alternate patch now, and will try it out in
 an hour or so.

 ---
  src/qemu/qemu_driver.c |   10 --
  1 files changed, 8 insertions(+), 2 deletions(-)

 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index d9e35be..1b55eb1 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -2429,6 +2429,7 @@ qemuOpenFile(struct qemud_driver *driver, const char 
 *path, int oflags,
  bool bypass_security = false;
  unsigned int vfoflags = 0;
  int fd = -1;
 +int path_shared = virStorageFileIsSharedFS(path);
  uid_t uid = getuid();
  gid_t gid = getgid();
  
 @@ -2437,7 +2438,12 @@ qemuOpenFile(struct qemud_driver *driver, const char 
 *path, int oflags,
   * in the failure case */
  if (oflags  O_CREAT) {
  need_unlink = true;
 -vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
 +
 +/* Don't force chown on network-shared FS
 + * as it is likely to fail. */
 +if (path_shared = 0 || driver-dynamicOwnership)
 +vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
 +
  if (stat(path, sb) == 0) {
  is_reg = !!S_ISREG(sb.st_mode);
  /* If the path is regular file which exists
 @@ -2475,7 +2481,7 @@ qemuOpenFile(struct qemud_driver *driver, const char 
 *path, int oflags,
  }
  
  /* On Linux we can also verify the FS-type of the directory. */
 -switch (virStorageFileIsSharedFS(path)) {
 +switch (path_shared) {
  case 1:
 /* it was on a network share, so we'll continue
  * as outlined above
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


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


Re: [libvirt] [test-API PATCH 1/4] filter: new class for filter or extract data

2012-04-12 Thread Martin Kletzander
On 04/11/2012 04:04 PM, Guannan Ren wrote:
 activityfilter.py
 ---
  activityfilter.py |   74 
 +
  1 files changed, 74 insertions(+), 0 deletions(-)
  create mode 100644 activityfilter.py
 
 diff --git a/activityfilter.py b/activityfilter.py
 new file mode 100644
 index 000..d99d690
 --- /dev/null
 +++ b/activityfilter.py
 @@ -0,0 +1,74 @@
 +#!/usr/bin/env python
 +#
 +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
 +#
 +# libvirt-test-API is free software: you can redistribute it and/or modify it
 +# under the terms of the GNU General Public License as published by
 +# the Free Software Foundation, either version 2 of the License, or
 +# (at your option) any later version. This program is distributed in
 +# the hope that it will be useful, but WITHOUT ANY WARRANTY; without
 +# even the implied warranties of TITLE, NON-INFRINGEMENT,
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 +#
 +# The GPL text is available in the file COPYING that accompanies this
 +# distribution and at http://www.gnu.org/licenses.
 +#
 +
 +
 +class Filter(object):
 +filter activity list to form various data list
 +def __init__(self, activities_list):
 +self.testcase_keys = []
 +for activity in activities_list:
 +for testcase in activity:
 +testcases_key = testcase.keys()
 +self.testcase_keys += testcases_key
 +
 +def unique_testcase_cleansuffix(self):
 +get a list of module:testcase from activities_list
 +   eliminate duplicate items, with 'module:testcase_clean'
 +
 +keylist_clean = self._keylist_cleanappended_without_sleep()
 +return list(set(keylist_clean))
 +
 +def unique_testcases(self):
 + get a list of module:testcase from activities_list
 +eliminate duplicate items
 +
 +keylist = self._keylist_without_sleep_clean()
 +return list(set(keylist))
 +
 +def _keylist_without_sleep_clean(self):
 + filter out 'clean' and 'sleep' flag
 +to generate a list of testcases
 +
 +keylist = []
 +for key in self.testcase_keys:
 +key = key.lower()
 +if key == 'clean' or key == 'sleep':
 +continue
 +
 +keylist.append(key)
 +
 +return keylist
 +
 +def _keylist_cleanappended_without_sleep(self):
 + remove 'sleep' flag, and append ':_clean' to
 +the previous testcase to form a new element
 +
 +keylist_clean = []
 +prev_casename = ''
 +
 +for key in self.testcase_keys:
 +key = key.lower()
 +if key == 'sleep':
 +continue
 +
 +if key == 'clean':
 +keylist_clean.append(prev_casename + :_clean)
 +continue
 +
 +prev_casename = key
 +keylist_clean.append(prev_casename)
 +
 +return keylist_clean

This looks ok, ACK.
I'm looking at the rest now.

Martin

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


Re: [libvirt] [test-API PATCH 2/4] cfgcheck: new class implement testcase config file checking

2012-04-12 Thread Martin Kletzander
On 04/11/2012 04:04 PM, Guannan Ren wrote:
 casecfgcheck.py
 ---
  casecfgcheck.py |   66 
 +++
  1 files changed, 66 insertions(+), 0 deletions(-)
  create mode 100644 casecfgcheck.py
 
 diff --git a/casecfgcheck.py b/casecfgcheck.py
 new file mode 100644
 index 000..3c4696d
 --- /dev/null
 +++ b/casecfgcheck.py
 @@ -0,0 +1,66 @@
 +#!/usr/bin/env python
 +#
 +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
 +#
 +# libvirt-test-API is free software: you can redistribute it and/or modify it
 +# under the terms of the GNU General Public License as published by
 +# the Free Software Foundation, either version 2 of the License, or
 +# (at your option) any later version. This program is distributed in
 +# the hope that it will be useful, but WITHOUT ANY WARRANTY; without
 +# even the implied warranties of TITLE, NON-INFRINGEMENT,
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 +#
 +# The GPL text is available in the file COPYING that accompanies this
 +# distribution and at http://www.gnu.org/licenses.
 +#
 +
 +import proxy
 +
 +class CaseCfgCheck(object):
 +validate the options in testcase config file
 +def __init__(self, unique_testcases, activities_list):
 +self.unique_testcases = unique_testcases
 +
 +# XXX to check the first testcase list in activities_list
 +self.activitie = activities_list[0]
 +
 +proxy_obj = proxy.Proxy(self.unique_testcases)
 +self.case_params = proxy_obj.get_params_variables()

typo:
s/activitie/activity/

plus I'm not sure if we can be sure that all the activities are the same
in this call (if it's just a multiplication of one array)? On other
places (like 'libvirt-test-api.py') we check for all members in the array.

 +
 +def check(self):
 +check options to each testcase in case config file
 +case_number = 0
 +error_flag = 0
 +passed_testcase = []
 +for testcase in self.activitie:

second typo or copy paste error :)
s/activitie/activity/

 +case_number += 1
 +if testcase in passed_testcase:
 +continue
 +
 +testcase_name = testcase.keys()[0]
 +actual_params = testcase.values()[0]

I don't quite understand what are you trying to achieve here. The order
of keys and values in python dict cannot be guaranteed if I'm not wrong.
And even if yes, that would mean you are checking just for the first
testcase, I guess. Or maybe I just don't get this right.

 +
 +required_params, optional_params = 
 self.case_params[testcase_name]
 +ret = self._check_params(required_params, optional_params, 
 actual_params)
 +if ret:
 +error_flag = 1
 +print the No.%s : %s\n % (case_number, testcase_name)
 +
 +passed_testcase.append(testcase)
 +
 +if error_flag:
 +return 1
 +return 0
 +
 +def _check_params(self, required_params, optional_params, actual_params):
 +for p in required_params:
 +if p not in actual_params.keys():
 +print Parameter %s is required % p
 +return 1
 +
 +for p in actual_params.keys():
 +if p not in required_params and p not in optional_params:
 +print Unknown parameter '%s' % p
 +return 1
 +
 +return 0

Martin

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


Re: [libvirt] [test-API PATCH 0/4]Add feature to check case file before running

2012-04-12 Thread Peter Krempa
I don't think pushing this series without a review was a good idea. You 
actualy broke all of the tests in the repos/ as you didn't do the 
modifications to the parameter checking algorithm in a way that didn't 
require modification of the tests, neither did you change the tests to 
cope with the new code. The result is now:


exception.TestCaseError: 'required_params or optional_params not found 
in interface:destroy'


or similar for every test case.

Peter

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


[libvirt] [PATCH] qemu_agent: Report error class at least

2012-04-12 Thread Michal Privoznik
Currently, qemu GA is not providing 'desc' field for errors like
we are used to from qemu monitor. Therefore, we fall back to this
general 'unknown error' string. However, GA is reporting 'class' which
is not perfect, but much more helpful than generic error string.
Thus we should fall back to class firstly and if even no class
is presented, then we can fall back to that generic string.

Before this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': unknown QEMU command error

After this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': CommandNotFound
---
 src/qemu/qemu_agent.c |   14 ++
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index b759b7f..decfd0e 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1035,19 +1035,17 @@ static const char *
 qemuAgentStringifyError(virJSONValuePtr error)
 {
 const char *klass = virJSONValueObjectGetString(error, class);
-const char *detail = NULL;
+const char *detail = virJSONValueObjectGetString(error, desc);
 
 /* The QMP 'desc' field is usually sufficient for our generic
- * error reporting needs.
+ * error reporting needs. However, older agents did not provide
+ * any 'desc'. Reporting 'class' is not perfect but better
+ * than bare 'unknown error'.
  */
-if (klass)
-detail = virJSONValueObjectGetString(error, desc);
-
-
-if (!detail)
+if (!detail  !klass)
 detail = unknown QEMU command error;
 
-return detail;
+return detail ? detail : klass;
 }
 
 static const char *
-- 
1.7.8.5

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


Re: [libvirt] [PATCH] qemu_agent: Report error class at least

2012-04-12 Thread Daniel P. Berrange
On Thu, Apr 12, 2012 at 02:06:21PM +0200, Michal Privoznik wrote:
 Currently, qemu GA is not providing 'desc' field for errors like
 we are used to from qemu monitor. Therefore, we fall back to this
 general 'unknown error' string. However, GA is reporting 'class' which
 is not perfect, but much more helpful than generic error string.
 Thus we should fall back to class firstly and if even no class
 is presented, then we can fall back to that generic string.
 
 Before this patch:
 virsh # dompmsuspend --target mem f16
 error: Domain f16 could not be suspended
 error: internal error unable to execute QEMU command
 'guest-suspend-ram': unknown QEMU command error
 
 After this patch:
 virsh # dompmsuspend --target mem f16
 error: Domain f16 could not be suspended
 error: internal error unable to execute QEMU command
 'guest-suspend-ram': CommandNotFound
 ---
  src/qemu/qemu_agent.c |   14 ++
  1 files changed, 6 insertions(+), 8 deletions(-)
 
 diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
 index b759b7f..decfd0e 100644
 --- a/src/qemu/qemu_agent.c
 +++ b/src/qemu/qemu_agent.c
 @@ -1035,19 +1035,17 @@ static const char *
  qemuAgentStringifyError(virJSONValuePtr error)
  {
  const char *klass = virJSONValueObjectGetString(error, class);
 -const char *detail = NULL;
 +const char *detail = virJSONValueObjectGetString(error, desc);
  
  /* The QMP 'desc' field is usually sufficient for our generic
 - * error reporting needs.
 + * error reporting needs. However, older agents did not provide
 + * any 'desc'. Reporting 'class' is not perfect but better
 + * than bare 'unknown error'.
   */
 -if (klass)
 -detail = virJSONValueObjectGetString(error, desc);
 -
 -
 -if (!detail)
 +if (!detail  !klass)
  detail = unknown QEMU command error;
  
 -return detail;
 +return detail ? detail : klass;
  }

You can get a list of all 'class' names that QEMU currently supports
from qerror.h.  As we do for VIR_ERR_X constants, you should create
a mapping from QEMU class names, to error message strings.

Even better, share this mapping between the agent  json monitor code
so we can improve error messages in both cases.

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

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


[libvirt] virsh list hangs

2012-04-12 Thread Qian Zhang
Hi,

I am using RHEL 6.2 64bit, and the libvirt shipped in it is
libvirt-0.9.4-23.el6.x86_64. I found sometimes the command virsh
list hangs forever, and same issue for virt-manager which is always
in Connecting status. But after restarting the libvirtd service,
this issue is gone.

Is this a bug of libvirt? Any help will be appreciated.


Thanks,
Qian

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


Re: [libvirt] [libvirt-glib] Explicitly link conn-test against libvirt-gconfig libvirt-glib

2012-04-12 Thread Guido Günther
On Wed, Apr 11, 2012 at 01:41:20PM +0100, Daniel P. Berrange wrote:
 On Wed, Apr 11, 2012 at 01:54:27PM +0200, Guido Günther wrote:
  On Tue, Apr 10, 2012 at 09:36:57PM +0100, Daniel P. Berrange wrote:
   On Tue, Apr 10, 2012 at 10:29:59PM +0200, Guido Günther wrote:
otherwise the build fails with:

$ CCLD   conn-test
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_domain_new_from_xml@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_set_error@LIBVIRT_GLIB_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_network_new_from_xml@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_storage_pool_new_from_xml@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_init_check@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_domain_get_type@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_object_to_xml@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_event_register@LIBVIRT_GLIB_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_init_check@LIBVIRT_GLIB_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_domain_get_devices@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_storage_vol_new_from_xml@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_secret_new_from_xml@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_interface_new_from_xml@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_domain_disk_get_type@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_domain_disk_get_target_dev@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_domain_interface_get_type@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_node_device_new_from_xml@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_domain_device_get_type@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_object_get_type@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_error_new_literal@LIBVIRT_GLIB_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_domain_snapshot_new_from_xml@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_network_filter_new_from_xml@LIBVIRT_GCONFIG_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_set_error_literal@LIBVIRT_GLIB_0.0.4'
../libvirt-gobject/.libs/libvirt-gobject-1.0.so: undefined reference to 
`gvir_config_domain_interface_get_ifname@LIBVIRT_GCONFIG_0.0.4'
collect2: ld returned 1 exit status
---
 examples/Makefile.am |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/examples/Makefile.am b/examples/Makefile.am
index b77076d..37a8447 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -25,6 +25,8 @@ conn_test_SOURCES = \
conn-test.c
 conn_test_LDADD = \
../libvirt-gobject/libvirt-gobject-1.0.la \
+   ../libvirt-gconfig/libvirt-gconfig-1.0.la \
+   ../libvirt-glib/libvirt-glib-1.0.la \
$(LIBVIRT_LIBS) \
$(GLIB2_LIBS) \
$(GOBJECT2_LIBS)
   
   I don't think this is right. libvirt-gobject-1.0.la already specifies
   a dependancy on libvirt-gconfig-1.0.la and libvirt-glib-1.0.la, so
   we should not have to duplicate that information:
   
   
   $ grep dependency libvirt-gobject/libvirt-gobject-1.0.la 
   # Linker flags that can not go in dependency_libs.
   dependency_libs=' -lgio-2.0 -lgmodule-2.0 
   /home/berrange/src/virt/libvirt-glib/libvirt-glib/libvirt-glib-1.0.la 
   -lvirt -ldl 
   /home/berrange/src/virt/libvirt-glib/libvirt-gconfig/libvirt-gconfig-1.0.la
-lgobject-2.0 -lgthread-2.0 -lrt -lglib-2.0 -lxml2'
  
  These are not emitted by Debian's libtool motivated by:
  
  http://lists.debian.org/debian-devel/2009/08/msg00783.html
  

Re: [libvirt] virsh list hangs

2012-04-12 Thread Michal Privoznik
On 12.04.2012 10:40, Qian Zhang wrote:
 Hi,
 
 I am using RHEL 6.2 64bit, and the libvirt shipped in it is
 libvirt-0.9.4-23.el6.x86_64. I found sometimes the command virsh
 list hangs forever, and same issue for virt-manager which is always
 in Connecting status. But after restarting the libvirtd service,
 this issue is gone.
 
 Is this a bug of libvirt? Any help will be appreciated.

I think I saw a bug in recent history about this, but now I can't find
it. Anyway, please file a bug: https://bugzilla.redhat.com/
And attach debug logs. You can obtain them by editing
/etc/libvirt/libvirtd.conf and setting:

log_level=1
log_filters=
log_outputs=1:file:/var/log/libvirtd.log

For more information follow up http://libvirt.org/logging.html;

For client log (we refer to virsh, virt-manager, ... as clients) you can
set environment variables:

LIBVIRT_DEBUG=1 LIBVIRT_LOG_OUTPUTS=1:file:client.log virt-manager
--no-fork

Then attach /var/log/libvirtd.log and client.log to the bugzilla.

Michal

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


Re: [libvirt] [PATCH] qemuOpenFile: Don't force chown on NFS

2012-04-12 Thread Michal Privoznik
On 12.04.2012 13:16, Laine Stump wrote:
 On 04/11/2012 04:21 PM, Laine Stump wrote:
 ACK to the idea, but NACK to the exact placement of the fix.
 
 
 On further examination (and actually doing a couple tests), I withdraw
 my NACK on the placement. I had mixed up usage of qemuOpenFile and
 virFileOpen in my memory - qemuOpenFile already hardly ever cares about
 the ownership of a file it opens, and even when it does, it always wants
 it to be owned by root (well, actually getuid()).
 
 So, although putting the fix in the place I suggested does work, my
 reasoning was flawed and your original position also works properly, as
 well as keeping the logic for setting the FORCE_OWNER flag in one place
 - ACK.
 
 Sorry for the noise :-)

Not at all. In fact, I find this useful as somebody else has tested it.

Pushed. Thanks.

Michal

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


Re: [libvirt] [PATCH] qemu_agent: Report error class at least

2012-04-12 Thread Michal Privoznik
On 12.04.2012 14:14, Daniel P. Berrange wrote:
 On Thu, Apr 12, 2012 at 02:06:21PM +0200, Michal Privoznik wrote:
 Currently, qemu GA is not providing 'desc' field for errors like
 we are used to from qemu monitor. Therefore, we fall back to this
 general 'unknown error' string. However, GA is reporting 'class' which
 is not perfect, but much more helpful than generic error string.
 Thus we should fall back to class firstly and if even no class
 is presented, then we can fall back to that generic string.

 Before this patch:
 virsh # dompmsuspend --target mem f16
 error: Domain f16 could not be suspended
 error: internal error unable to execute QEMU command
 'guest-suspend-ram': unknown QEMU command error

 After this patch:
 virsh # dompmsuspend --target mem f16
 error: Domain f16 could not be suspended
 error: internal error unable to execute QEMU command
 'guest-suspend-ram': CommandNotFound
 ---
  src/qemu/qemu_agent.c |   14 ++
  1 files changed, 6 insertions(+), 8 deletions(-)

 diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
 index b759b7f..decfd0e 100644
 --- a/src/qemu/qemu_agent.c
 +++ b/src/qemu/qemu_agent.c
 @@ -1035,19 +1035,17 @@ static const char *
  qemuAgentStringifyError(virJSONValuePtr error)
  {
  const char *klass = virJSONValueObjectGetString(error, class);
 -const char *detail = NULL;
 +const char *detail = virJSONValueObjectGetString(error, desc);
  
  /* The QMP 'desc' field is usually sufficient for our generic
 - * error reporting needs.
 + * error reporting needs. However, older agents did not provide
 + * any 'desc'. Reporting 'class' is not perfect but better
 + * than bare 'unknown error'.
   */
 -if (klass)
 -detail = virJSONValueObjectGetString(error, desc);
 -
 -
 -if (!detail)
 +if (!detail  !klass)
  detail = unknown QEMU command error;
  
 -return detail;
 +return detail ? detail : klass;
  }
 
 You can get a list of all 'class' names that QEMU currently supports
 from qerror.h.  As we do for VIR_ERR_X constants, you should create
 a mapping from QEMU class names, to error message strings.
 
 Even better, share this mapping between the agent  json monitor code
 so we can improve error messages in both cases.
 
 Regards,
 Daniel

I don't think this should be shared; as written in commit message,
in case of json monitor we get 'desc' which fulfills translation
from 'class' to error message. Moreover, the 'desc' field already
contains correct values, e.g.:

#define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': 
%s, 'feature': %s } }

{execute:unknown command}
{error: {class: CommandNotFound, desc: The command unknown command has 
not been found, data: {name: unknown command}}}


However, things change rapidly with GA:
{execute:unknown command}
{error: {class: CommandNotFound, data: {name: unknown command}}}

Therefore I see point in having translation table just for GA.
In fact, such table doesn't need to cover all error codes from qerror.h
only those used by GA:

~/work/qemu.git $ grep -r -o -e QERR_[A-Z_]* qga/ qapi* | cut -d':' -f 2 | sort 
| uniq
QERR_BUFFER_OVERRUN
QERR_COMMAND_DISABLED
QERR_COMMAND_NOT_FOUND
QERR_FD_NOT_FOUND
QERR_INVALID_PARAMETER
QERR_INVALID_PARAMETER_TYPE
QERR_INVALID_PARAMETER_VALUE
QERR_OPEN_FILE_FAILED
QERR_QGA_COMMAND_FAILED
QERR_QGA_LOGGING_DISABLED
QERR_QMP_BAD_INPUT_OBJECT
QERR_QMP_BAD_INPUT_OBJECT_MEMBER
QERR_QMP_EXTRA_MEMBER
QERR_UNDEFINED_ERROR
QERR_UNSUPPORTED

Michal

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


Re: [libvirt] [test-API PATCH 0/4]Add feature to check case file before running

2012-04-12 Thread Guannan Ren

On 04/12/2012 07:53 PM, Peter Krempa wrote:
I don't think pushing this series without a review was a good idea. 
You actualy broke all of the tests in the repos/ as you didn't do the 
modifications to the parameter checking algorithm in a way that didn't 
require modification of the tests, neither did you change the tests to 
cope with the new code. The result is now:


exception.TestCaseError: 'required_params or optional_params not found 
in interface:destroy'


or similar for every test case.

Peter



Yes, sorry about this.
There is some new feature and cleanup work on my hand, I don't know
the exact time to get review.
The error is generated by framework.  the part job of framework is 
done.
The cleanup work on testcase is ongoing,  I am sure that I will 
finish the work today.


 Guannan Ren

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


[libvirt] [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC

2012-04-12 Thread Stefan Bader
This is a re-send as there was some positive feedback but the
patch itself made it into the repo.

-Stefan

From a3198c5c1ae8908818f6c0f0df4237dbe5ddeec7 Mon Sep 17 00:00:00 2001
From: Stefan Bader stefan.ba...@canonical.com
Date: Thu, 12 Apr 2012 15:32:41 +0200
Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC

When using the xm/xend stack to manage instances there is a bug
that causes the emulated interfaces to be unusable when the vif
config contains type=ioemu (MAC address all zero).

The current code already has a special quirk to not use this
keyword if no specific model is given for the emulated NIC
(defaulting to rtl8139).
Essentially it works because regardless of the type argument,
the Xen stack always creates emulated and paravirt interfaces and
lets the guest decide which one to use. So neither xl nor xm stack
actually require the type keyword for emulated NICs.

Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
 src/xenxs/xen_sxpr.c |4 +++-
 src/xenxs/xen_xm.c   |4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index e1bbd76..71602fa 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -2012,7 +2012,9 @@ xenFormatSxprNet(virConnectPtr conn,
 }
 else {
 virBufferEscapeSexpr(buf, (model '%s'), def-model);
-virBufferAddLit(buf, (type ioemu));
+   /* See above. Also needed when model is specified. */
+if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
+virBufferAddLit(buf, (type ioemu));
 }
 
 if (!isAttach)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index d65e97a..e072599 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1394,7 +1394,9 @@ static int xenFormatXMNet(virConnectPtr conn,
 }
 else {
 virBufferAsprintf(buf, ,model=%s, net-model);
-virBufferAddLit(buf, ,type=ioemu);
+   /* See above. Also needed if model is specified. */
+if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
+virBufferAddLit(buf, ,type=ioemu);
 }
 
 if (net-ifname)
-- 
1.7.9.5

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


[libvirt] [PATCH] xend_internal: Use domain/status for shutdown check

2012-04-12 Thread Stefan Bader
As promised this version does keep the domid  0 check in
order to be clearly keeping the old behavior.

-Stefan

From 18d398d98dc0dc2d9148ffb8673c651248d1bca5 Mon Sep 17 00:00:00 2001
From: Stefan Bader stefan.ba...@canonical.com
Date: Thu, 12 Apr 2012 09:59:56 +
Subject: [PATCH] xend_internal: Use domain/status for shutdown check

On newer xend (v3.x and after) there is no state and domid reported
for inactive domains. When initially creating connections this is
handled in various places by assigning domain-id = -1.
But once an instance has been running, the id is set to the current
domain id. And it does not change when the instance is shut down.
So when querying the domain info, the hypervisor driver, which gets
asked first will indicate it cannot find information, then the
xend driver is asked and will set the status to NOSTATE because it
checks for the -1 domain id.
Checking domain/status for 0 seems to be more reliable for that.

One note: I am not sure whether the domain-id also should get set
back to -1 whenever any sub-driver thinks the instance is no longer
running.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=746007
BugLink: http://bugs.launchpad.net/bugs/929626

[v2: Keep old id check just in case]
Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
 src/xen/xend_internal.c |   11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 6526af4..f1aa9b6 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -989,9 +989,14 @@ sexpr_to_xend_domain_state(virDomainPtr domain, const 
struct sexpr *root)
 state = VIR_DOMAIN_BLOCKED;
 else if (strchr(flags, 'r'))
 state = VIR_DOMAIN_RUNNING;
-} else if (domain-id  0) {
-/* Inactive domains don't have a state reported, so
-   mark them SHUTOFF, rather than NOSTATE */
+} else if (domain-id  0 || sexpr_int(root, domain/status) == 0) {
+/* As far as I can see the domain-id is a bad sign for checking
+ * inactive domains as this is inaccurate after the domain has
+ * been running once. However domain/status from xend seems to
+ * be always present and 0 for inactive domains.
+ * (keeping the check for id  0 to be extra safe about backward
+ * compatibility)
+ */
 state = VIR_DOMAIN_SHUTOFF;
 }
 
-- 
1.7.9.5

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


[libvirt] [PATCH v2] qemu_agent: Report error class at least

2012-04-12 Thread Michal Privoznik
Currently, qemu GA is not providing 'desc' field for errors like
we are used to from qemu monitor. Therefore, we fall back to this
general 'unknown error' string. However, GA is reporting 'class' which
is not perfect, but much more helpful than generic error string.
Thus we should fall back to class firstly and if even no class
is presented, then we can fall back to that generic string.

Before this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': unknown QEMU command error

After this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': The command has not been found
---
 src/qemu/qemu_agent.c |   37 +++--
 1 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index b759b7f..bc4ceff 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1025,6 +1025,40 @@ cleanup:
 return ret;
 }
 
+static const char *
+qemuAgentStringifyErrorClass(const char *klass)
+{
+if (STREQ_NULLABLE(klass, BufferOverrun))
+return Buffer overrun;
+else if (STREQ_NULLABLE(klass, CommandDisabled))
+return The command has been disabled for this instance;
+else if (STREQ_NULLABLE(klass, CommandNotFound))
+return The command has not been found;
+else if (STREQ_NULLABLE(klass, FdNotFound))
+return File descriptor not found;
+else if (STREQ_NULLABLE(klass, InvalidParameter))
+return Invalid parameter;
+else if (STREQ_NULLABLE(klass, InvalidParameterType))
+return Invalid parameter type;
+else if (STREQ_NULLABLE(klass, InvalidParameterValue))
+return Invalid parameter value;
+else if (STREQ_NULLABLE(klass, OpenFileFailed))
+return Cannot open file;
+else if (STREQ_NULLABLE(klass, QgaCommandFailed))
+return Guest agent command failed;
+else if (STREQ_NULLABLE(klass, QMPBadInputObjectMember))
+return Bad QMP input object member;
+else if (STREQ_NULLABLE(klass, QMPExtraInputObjectMember))
+return Unexpected extra object member;
+else if (STREQ_NULLABLE(klass, UndefinedError))
+return An undefined error has occurred;
+else if (STREQ_NULLABLE(klass, Unsupported))
+return this feature or command is not currently supported;
+else if (klass)
+return klass;
+else
+return unknown QEMU command error;
+}
 
 /* Ignoring OOM in this method, since we're already reporting
  * a more important error
@@ -1043,9 +1077,8 @@ qemuAgentStringifyError(virJSONValuePtr error)
 if (klass)
 detail = virJSONValueObjectGetString(error, desc);
 
-
 if (!detail)
-detail = unknown QEMU command error;
+detail = qemuAgentStringifyErrorClass(klass);
 
 return detail;
 }
-- 
1.7.8.5

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


Re: [libvirt] virsh list hangs

2012-04-12 Thread Alex Jia
Here is a similar issues, please see Jiri's comment:
https://bugzilla.redhat.com/show_bug.cgi?id=797835#c5

Regards,
Alex


- Original Message -
From: Michal Privoznik mpriv...@redhat.com
To: Qian Zhang zhq527...@gmail.com
Cc: libvir-list@redhat.com
Sent: Thursday, April 12, 2012 8:49:21 PM
Subject: Re: [libvirt] virsh list hangs

On 12.04.2012 10:40, Qian Zhang wrote:
 Hi,
 
 I am using RHEL 6.2 64bit, and the libvirt shipped in it is
 libvirt-0.9.4-23.el6.x86_64. I found sometimes the command virsh
 list hangs forever, and same issue for virt-manager which is always
 in Connecting status. But after restarting the libvirtd service,
 this issue is gone.
 
 Is this a bug of libvirt? Any help will be appreciated.

I think I saw a bug in recent history about this, but now I can't find
it. Anyway, please file a bug: https://bugzilla.redhat.com/
And attach debug logs. You can obtain them by editing
/etc/libvirt/libvirtd.conf and setting:

log_level=1
log_filters=
log_outputs=1:file:/var/log/libvirtd.log

For more information follow up http://libvirt.org/logging.html;

For client log (we refer to virsh, virt-manager, ... as clients) you can
set environment variables:

LIBVIRT_DEBUG=1 LIBVIRT_LOG_OUTPUTS=1:file:client.log virt-manager
--no-fork

Then attach /var/log/libvirtd.log and client.log to the bugzilla.

Michal

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

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


Re: [libvirt] [test-API PATCH 2/4] cfgcheck: new class implement testcase config file checking

2012-04-12 Thread Guannan Ren

On 04/12/2012 07:38 PM, Martin Kletzander wrote:

On 04/11/2012 04:04 PM, Guannan Ren wrote:

 casecfgcheck.py
---
  casecfgcheck.py |   66 +++
  1 files changed, 66 insertions(+), 0 deletions(-)
  create mode 100644 casecfgcheck.py

diff --git a/casecfgcheck.py b/casecfgcheck.py
new file mode 100644
index 000..3c4696d
--- /dev/null
+++ b/casecfgcheck.py
@@ -0,0 +1,66 @@
+#!/usr/bin/env python
+#
+# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
+#
+# libvirt-test-API is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version. This program is distributed in
+# the hope that it will be useful, but WITHOUT ANY WARRANTY; without
+# even the implied warranties of TITLE, NON-INFRINGEMENT,
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+#
+# The GPL text is available in the file COPYING that accompanies this
+# distribution and athttp://www.gnu.org/licenses.
+#
+
+import proxy
+
+class CaseCfgCheck(object):
+validate the options in testcase config file
+def __init__(self, unique_testcases, activities_list):
+self.unique_testcases = unique_testcases
+
+# XXX to check the first testcase list in activities_list
+self.activitie = activities_list[0]
+
+proxy_obj = proxy.Proxy(self.unique_testcases)
+self.case_params = proxy_obj.get_params_variables()

typo:
s/activitie/activity/

plus I'm not sure if we can be sure that all the activities are the same
in this call (if it's just a multiplication of one array)? On other
places (like 'libvirt-test-api.py') we check for all members in the array.


+
+def check(self):
+check options to each testcase in case config file
+case_number = 0
+error_flag = 0
+passed_testcase = []
+for testcase in self.activitie:

second typo or copy paste error :)
s/activitie/activity/


   Thanks,  it is fixed and pushed.


+case_number += 1
+if testcase in passed_testcase:
+continue
+
+testcase_name = testcase.keys()[0]
+actual_params = testcase.values()[0]

I don't quite understand what are you trying to achieve here. The order
of keys and values in python dict cannot be guaranteed if I'm not wrong.
And even if yes, that would mean you are checking just for the first
testcase, I guess. Or maybe I just don't get this right.



 If time is permitted, please run python parser.py 
testcase.cfg

 It will output the data format after parsing.
 example:  run it using the command above.
#testcase.cfg
domain:testa
guestname
fedora16

 The data format is: [[{'domain:testa': {'guestname': 
'fedora16'}}]]
 The variable testcase refer to {'domain:testa': 
{'guestname': 'fedora16'}
 The variable activity refet to [{'domain:testa': 
{'guestname': 'fedora16'}]

 Note: the code just to check the first inner list here.

 then, we use 'testcase_name'(domain:testa) as the key to 
fetch two

 global variable that defined in testa.py
 one is required_params, the other is optional_params, 
then compare them

 with actual_params by invoking _check_params(...)

 Tips: the proxy.py will place the two references to the 
global params
 in a data format like {'domain:testa':[required_params, 
optional_params]}


 Guannan Ren





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


Re: [libvirt] [test-API PATCH 0/4]Add feature to check case file before running

2012-04-12 Thread Guannan Ren

On 04/12/2012 09:43 PM, Guannan Ren wrote:

On 04/12/2012 07:53 PM, Peter Krempa wrote:
I don't think pushing this series without a review was a good idea. 
You actualy broke all of the tests in the repos/ as you didn't do the 
modifications to the parameter checking algorithm in a way that 
didn't require modification of the tests, neither did you change the 
tests to cope with the new code. The result is now:


exception.TestCaseError: 'required_params or optional_params not 
found in interface:destroy'


or similar for every test case.

Peter



Yes, sorry about this.
There is some new feature and cleanup work on my hand, I don't know
the exact time to get review.
The error is generated by framework.  the part job of framework is 
done.
The cleanup work on testcase is ongoing,  I am sure that I will 
finish the work today.




The cleanup is done and pushed.
I only wan to send framework code here, the cleaning code in 
testcases is huge

and mechanical, maybe nobody likes seeing it :)
Sorry about the intact commit again.

Guannan Ren

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


[libvirt] Sys-Virt and libvirt, question

2012-04-12 Thread Jason Helfman
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Hi,

I was wondering if Sys-Virt needs to coincide with an update to libvirt?

A fellow committer at FreeBSD is running the latest Sys-Virt against the
latest libvirt and is getting this error:
http://meatwad.mouf.net/tb/errors/9-STABLE-amd64-FreeBSD/p5-Sys-Virt-0.9.10.log

Thanks!
- -jgh

- -- 
Jason Helfman
System Administrator
experts-exchange.com
http://www.experts-exchange.com/M_4830110.html
E4AD 7CF1 1396 27F6 79DD  4342 5E92 AD66 8C8C FBA5
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.18 (FreeBSD)

iF4EAREIAAYFAk+HDfYACgkQXpKtZoyM+6UidQD8CKAsKrz9+W4iTFxf8SJOkgtz
8HTX8kWhuEPnA65CqeoA/0p4ncg3j08GWFNT+K66bk/8BaAXIzIpZifivuDN9bxM
=CbDk
-END PGP SIGNATURE-

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


Re: [libvirt] [PATCH] qemu, util fix netlink callback registration for migration

2012-04-12 Thread Laine Stump
On 03/29/2012 07:15 AM, D. Herrendoerfer wrote:
 From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name

 This patch adds a netlink callback when migrating a VEPA enabled
 virtual machine.
 It fixes a Bug where a VM would not request a port association when
 it was cleared by lldpad.
 This patch requires the latest git version of lldpad to work. 

 Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
 ---
  src/qemu/qemu_migration.c   |6 ++
  src/util/virnetdevmacvlan.c |   14 +-
  src/util/virnetdevmacvlan.h |8 
  3 files changed, 27 insertions(+), 1 deletions(-)


Sorry for the delay in pushing this. When it came in we were well into
the pre-release freeze for 0.9.11, and I didn't want to take the chance
of breaking the build, and by the time the release was done I'd
forgotten it.

Note that this patch actually would have broken the build on some
platforms as delivered, since you forgot to add
virNetDevMacVLanVPortProfileRegisterCallback to libvirt_private.syms
(the problems only show up if you build with driver modules enabled).
ACK with that omission corrected; I've done that and pushed the result.

Something I noticed this time when looking at the code - it seems like
clients might be getting registered at times when they aren't needed;
specifically - as I understand it, lldpad is only a part of the picture
for 802..1Qbg, is that correct? Unless I'm reading the code wrong,
callbacks are registered for any direct interface that has a port
profile, regardless of type. That's most likely harmless, but if my
suppositions are correct, we should probably clean this up so it only
registers for those interfaces that are actually going to get an event.




 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index 77d40c0..7a8a7c4 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -2654,6 +2654,12 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr 
 def) {
 def-uuid,
 
 VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false)  0)
  goto err_exit;
 +
 +if (virNetDevMacVLanVPortProfileRegisterCallback(net-ifname, 
 net-mac,
 + 
 virDomainNetGetActualDirectDev(net), def-uuid,
 + 
 virDomainNetGetActualVirtPortProfile(net),
 + 
 VIR_NETDEV_VPORT_PROFILE_OP_CREATE))
 +goto err_exit;
  }
  last_good_net = i;
  }
 diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
 index 90888b0..b259e00 100644
 --- a/src/util/virnetdevmacvlan.c
 +++ b/src/util/virnetdevmacvlan.c
 @@ -769,7 +769,7 @@ virNetDevMacVLanVPortProfileDestroyCallback(int watch 
 ATTRIBUTE_UNUSED,
  virNetlinkCallbackDataFree((virNetlinkCallbackDataPtr)opaque);
  }
  
 -static int
 +int
  virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname,
   const unsigned char *macaddress,
   const char *linkdev,
 @@ -1125,4 +1125,16 @@ int virNetDevMacVLanRestartWithVPortProfile(const char 
 *cr_ifname ATTRIBUTE_UNUS
   _(Cannot create macvlan devices on this 
 platform));
  return -1;
  }
 +
 +int virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname 
 ATTRIBUTE_UNUSED,
 + const unsigned char *macaddress 
 ATTRIBUTE_UNUSED,
 + const char *linkdev 
 ATTRIBUTE_UNUSED,
 + const unsigned char *vmuuid 
 ATTRIBUTE_UNUSED,
 + virNetDevVPortProfilePtr 
 virtPortProfile ATTRIBUTE_UNUSED,
 + enum virNetDevVPortProfileOp 
 vmOp ATTRIBUTE_UNUSED)
 +{
 +virReportSystemError(ENOSYS, %s,
 + _(Cannot create macvlan devices on this 
 platform));
 +return -1;
 +}
  #endif /* ! WITH_MACVTAP */
 diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h
 index 14640cf..2299f1d 100644
 --- a/src/util/virnetdevmacvlan.h
 +++ b/src/util/virnetdevmacvlan.h
 @@ -84,4 +84,12 @@ int virNetDevMacVLanRestartWithVPortProfile(const char 
 *cr_ifname,
  ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
  ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;
  
 +int virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname,
 + const unsigned char *macaddress 
 ,
 + const char *linkdev,
 + const unsigned char *vmuuid,
 + virNetDevVPortProfilePtr 
 

Re: [libvirt] Sys-Virt and libvirt, question

2012-04-12 Thread Daniel P. Berrange
On Thu, Apr 12, 2012 at 10:16:39AM -0700, Jason Helfman wrote:
 Hi,
 
 I was wondering if Sys-Virt needs to coincide with an update to libvirt?
 
 A fellow committer at FreeBSD is running the latest Sys-Virt against the
 latest libvirt and is getting this error:
 http://meatwad.mouf.net/tb/errors/9-STABLE-amd64-FreeBSD/p5-Sys-Virt-0.9.10.log

It does need an update to cover the new APIs, but any existing version
should still build just fine. I don't really know why you see that error
though - i think it is unrelated to the version number


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] Pull DBus event code out into common area

2012-04-12 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The policy kit and HAL node device drivers both require a
DBus connection. The HAL device code further requires that
the DBus connection is integrated with the event loop and
provides such glue logic itself.

The forthcoming FirewallD integration also requires a
dbus connection with event loop integration. Thus we need
to pull the current event loop glue out of the HAL driver.

Thus we create src/util/virdbus.{c,h} files. This contains
just one method virDBusGetSystemBus() which obtains a handle
to the single shared system bus instance, with event glue
automagically setup.

NB, I have not actually tested this on a system with HAL or
PolicyKit-0 installed, so it may well not compile. Hopefully
someone on list has a suitable system where they can test
those two. If not, I'll install a VM next week to test it in.

---
 .gitignore |6 ++--
 configure.ac   |3 +-
 daemon/Makefile.am |3 +-
 daemon/libvirtd.c  |4 ---
 src/Makefile.am|3 +-
 src/rpc/virnetserver.c |6 
 src/rpc/virnetserver.h |4 ---
 src/util/virdbus.c |   68 ++-
 8 files changed, 40 insertions(+), 57 deletions(-)

diff --git a/.gitignore b/.gitignore
index 5aa9c9b..14a21d0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -48,12 +48,12 @@
 /daemon/*_dispatch.h
 /daemon/libvirt_qemud
 /daemon/libvirtd
-/daemon/libvirtd.init
-/daemon/libvirtd.service
 /daemon/libvirtd*.logrotate
 /daemon/libvirtd.8
 /daemon/libvirtd.8.in
+/daemon/libvirtd.init
 /daemon/libvirtd.pod
+/daemon/libvirtd.service
 /docs/devhelp/libvirt.devhelp
 /docs/hvsupport.html.in
 /docs/libvirt-api.xml
@@ -118,6 +118,7 @@
 /tests/eventtest
 /tests/hashtest
 /tests/jsontest
+/tests/libvirtdconftest
 /tests/networkxml2argvtest
 /tests/nodeinfotest
 /tests/nwfilterxml2xmltest
@@ -150,7 +151,6 @@
 /tests/vmx2xmltest
 /tests/xencapstest
 /tests/xmconfigtest
-/tests/libvirtdconftest
 /tools/*.[18]
 /tools/libvirt-guests.init
 /tools/virsh
diff --git a/configure.ac b/configure.ac
index 3863119..f49b620 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1116,9 +1116,8 @@ if test $with_dbus = yes || test $with_dbus = 
check ; then
AC_MSG_ERROR([You must install DBus = $DBUS_REQUIRED to compile 
libvirt])
  fi])
 fi
-echo  $with_dbus 
+
 if test $with_dbus = yes ; then
-echo  $with_dbus 
   AC_DEFINE_UNQUOTED([HAVE_DBUS], 1, [enable communication with DBus])
 
   save_LIBS=$LIBS
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 5d9f5d7..24cce8f 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -94,7 +94,7 @@ libvirtd_SOURCES = $(DAEMON_SOURCES)
 #-D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L
 libvirtd_CFLAGS = \
$(LIBXML_CFLAGS) $(GNUTLS_CFLAGS) $(SASL_CFLAGS) \
-   $(XDR_CFLAGS) $(POLKIT_CFLAGS) \
+   $(XDR_CFLAGS) $(POLKIT_CFLAGS) $(DBUS_CFLAGS) \
$(WARN_CFLAGS) \
$(COVERAGE_CFLAGS) \
-DQEMUD_PID_FILE=\$(QEMUD_PID_FILE)\ \
@@ -108,6 +108,7 @@ libvirtd_LDADD =\
$(LIBXML_LIBS)  \
$(GNUTLS_LIBS)  \
$(SASL_LIBS)\
+   $(DBUS_LIBS)\
$(POLKIT_LIBS)
 
 if WITH_DTRACE_PROBES
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index ce931d4..b098f6a 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -812,7 +812,6 @@ int main(int argc, char **argv) {
 struct daemonConfig *config;
 bool privileged = geteuid() == 0 ? true : false;
 bool implicit_conf = false;
-bool use_polkit_dbus;
 char *run_dir = NULL;
 mode_t old_umask;
 
@@ -1008,8 +1007,6 @@ int main(int argc, char **argv) {
 goto cleanup;
 }
 
-use_polkit_dbus = config-auth_unix_rw == REMOTE_AUTH_POLKIT ||
-config-auth_unix_ro == REMOTE_AUTH_POLKIT;
 if (!(srv = virNetServerNew(config-min_workers,
 config-max_workers,
 config-prio_workers,
@@ -1018,7 +1015,6 @@ int main(int argc, char **argv) {
 config-keepalive_count,
 !!config-keepalive_required,
 config-mdns_adv ? config-mdns_name : NULL,
-use_polkit_dbus,
 remoteClientInitHook))) {
 ret = VIR_DAEMON_ERR_INIT;
 goto cleanup;
diff --git a/src/Makefile.am b/src/Makefile.am
index c6b7033..b8a19b4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1523,7 +1523,7 @@ libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(AM_LDFLAGS)
 libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \
$(LIBXML_LIBS) $(NUMACTL_LIBS) $(THREAD_LIBS) \
$(LIBNL_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
-   $(RT_LIBS) \
+   $(RT_LIBS) 

[libvirt] [PATCH] blockjob: add virsh blockjob --wait

2012-04-12 Thread Eric Blake
I'm tired of shell-scripting to wait for completion of a block pull,
when virsh can be taught to do the same.  I couldn't quite reuse
vshWatchJob, as this is not a case of a long-running command where
a second thread must be used to probe job status (at least, not unless
I make virsh start doing blocking waits for an event to fire), but it
served as inspiration for my simpler single-threaded loop.  There is
up to a half-second delay between sending SIGINT and the job being
aborted, but I didn't think it worth the complexity of a second thread
and use of poll() just to minimize that delay.

* tools/virsh.c (cmdBlockPull): Add new options to wait for
completion.
(blockJobImpl): Add argument.
(cmdBlockJob): Adjust caller.
* tools/virsh.pod (blockjob): Document new mode.
---
 tools/virsh.c   |  122 +-
 tools/virsh.pod |   11 -
 2 files changed, 119 insertions(+), 14 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 8ef57e0..c54add9 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7274,11 +7274,11 @@ repoll:
 if (pollfd.revents  POLLIN 
 saferead(pipe_fd, retchar, sizeof(retchar))  0 
 retchar == '0') {
-if (verbose) {
-/* print [100 %] */
-print_job_progress(label, 0, 1);
-}
-break;
+if (verbose) {
+/* print [100 %] */
+print_job_progress(label, 0, 1);
+}
+break;
 }
 goto cleanup;
 }
@@ -7295,8 +7295,9 @@ repoll:
 }

 GETTIMEOFDAY(curr);
-if ( timeout  ((int)(curr.tv_sec - start.tv_sec)  * 1000 + \
- (int)(curr.tv_usec - start.tv_usec) / 1000)  timeout 
* 1000 ) {
+if (timeout  (((int)(curr.tv_sec - start.tv_sec)  * 1000 +
+ (int)(curr.tv_usec - start.tv_usec) / 1000) 
+timeout * 1000)) {
 /* suspend the domain when migration timeouts. */
 vshDebug(ctl, VSH_ERR_DEBUG, %s timeout, label);
 if (timeout_func)
@@ -7519,7 +7520,8 @@ typedef enum {

 static int
 blockJobImpl(vshControl *ctl, const vshCmd *cmd,
-  virDomainBlockJobInfoPtr info, int mode)
+ virDomainBlockJobInfoPtr info, int mode,
+ virDomainPtr *pdom)
 {
 virDomainPtr dom = NULL;
 const char *name, *path;
@@ -7560,7 +7562,9 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
 }

 cleanup:
-if (dom)
+if (pdom  ret == 0)
+*pdom = dom;
+else if (dom)
 virDomainFree(dom);
 return ret;
 }
@@ -7580,15 +7584,109 @@ static const vshCmdOptDef opts_block_pull[] = {
 {bandwidth, VSH_OT_DATA, VSH_OFLAG_NONE, N_(Bandwidth limit in MB/s)},
 {base, VSH_OT_DATA, VSH_OFLAG_NONE,
  N_(path of backing file in chain for a partial pull)},
+{wait, VSH_OT_BOOL, 0, N_(wait for job to finish)},
+{verbose, VSH_OT_BOOL, 0, N_(with --wait, display the progress)},
+{timeout, VSH_OT_INT, VSH_OFLAG_NONE,
+ N_(with --wait, abort if pull exceeds timeout (in seconds))},
 {NULL, 0, 0, NULL}
 };

 static bool
 cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
 {
-if (blockJobImpl(ctl, cmd, NULL, VSH_CMD_BLOCK_JOB_PULL) != 0)
+virDomainPtr dom = NULL;
+bool ret = false;
+bool blocking = vshCommandOptBool(cmd, wait);
+bool verbose = vshCommandOptBool(cmd, verbose);
+int timeout = 0;
+struct sigaction sig_action;
+struct sigaction old_sig_action;
+sigset_t sigmask;
+struct timeval start;
+struct timeval curr;
+const char *path = NULL;
+bool quit = false;
+
+if (blocking) {
+if (vshCommandOptInt(cmd, timeout, timeout)  0) {
+if (timeout  1) {
+vshError(ctl, %s, _(migrate: Invalid timeout));
+return false;
+}
+
+/* Ensure that we can multiply by 1000 without overflowing. */
+if (timeout  INT_MAX / 1000) {
+vshError(ctl, %s, _(migrate: Timeout is too big));
+return false;
+}
+}
+if (vshCommandOptString(cmd, path, path)  0)
+return false;
+
+sigemptyset(sigmask);
+sigaddset(sigmask, SIGINT);
+
+intCaught = 0;
+sig_action.sa_sigaction = vshCatchInt;
+sigemptyset(sig_action.sa_mask);
+sigaction(SIGINT, sig_action, old_sig_action);
+
+GETTIMEOFDAY(start);
+} else if (verbose || vshCommandOptBool(cmd, timeout)) {
+vshError(ctl, %s, _(blocking control options require --wait));
 return false;
-return true;
+}
+
+if (blockJobImpl(ctl, cmd, NULL, VSH_CMD_BLOCK_JOB_PULL,
+ blocking ? dom : NULL) != 0)
+goto cleanup;
+
+while (blocking) {
+virDomainBlockJobInfo info;
+int 

Re: [libvirt] [PATCH] Pull DBus event code out into common area

2012-04-12 Thread Eric Blake
On 04/12/2012 01:14 PM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The policy kit and HAL node device drivers both require a
 DBus connection. The HAL device code further requires that
 the DBus connection is integrated with the event loop and
 provides such glue logic itself.
 
 The forthcoming FirewallD integration also requires a
 dbus connection with event loop integration. Thus we need
 to pull the current event loop glue out of the HAL driver.
 
 Thus we create src/util/virdbus.{c,h} files. This contains
 just one method virDBusGetSystemBus() which obtains a handle
 to the single shared system bus instance, with event glue
 automagically setup.
 
 NB, I have not actually tested this on a system with HAL or
 PolicyKit-0 installed, so it may well not compile. Hopefully
 someone on list has a suitable system where they can test
 those two. If not, I'll install a VM next week to test it in.
 
 ---
  .gitignore |6 ++--
  configure.ac   |3 +-
  daemon/Makefile.am |3 +-
  daemon/libvirtd.c  |4 ---
  src/Makefile.am|3 +-
  src/rpc/virnetserver.c |6 
  src/rpc/virnetserver.h |4 ---
  src/util/virdbus.c |   68 ++-
  8 files changed, 40 insertions(+), 57 deletions(-)

Did you forget to squash this into a prior patch?  The commit message
mentions a new file virdbus.h that I don't see here, and since I don't
have virdbus.c, this patch won't apply.

-- 
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] Pull DBus event code out into common area

2012-04-12 Thread Daniel P. Berrange
On Thu, Apr 12, 2012 at 02:05:44PM -0600, Eric Blake wrote:
 Did you forget to squash this into a prior patch?  The commit message
 mentions a new file virdbus.h that I don't see here, and since I don't
 have virdbus.c, this patch won't apply.

Urgh, yes. Squashed  resent

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 v2] Pull DBus event code out into common area

2012-04-12 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The policy kit and HAL node device drivers both require a
DBus connection. The HAL device code further requires that
the DBus connection is integrated with the event loop and
provides such glue logic itself.

The forthcoming FirewallD integration also requires a
dbus connection with event loop integration. Thus we need
to pull the current event loop glue out of the HAL driver.

Thus we create src/util/virdbus.{c,h} files. This contains
just one method virDBusGetSystemBus() which obtains a handle
to the single shared system bus instance, with event glue
automagically setup.
---
 .gitignore|6 +-
 configure.ac  |   37 ++-
 daemon/Makefile.am|3 +-
 daemon/libvirtd.c |4 -
 daemon/remote.c   |8 +-
 include/libvirt/virterror.h   |1 +
 src/Makefile.am   |   13 +--
 src/libvirt_dbus.syms |2 -
 src/node_device/node_device_hal.c |  143 ++
 src/rpc/virnetserver.c|   40 
 src/rpc/virnetserver.h|8 --
 src/util/virdbus.c|  201 +
 src/util/virdbus.h|   34 ++
 src/util/virterror.c  |3 +
 14 files changed, 296 insertions(+), 207 deletions(-)
 delete mode 100644 src/libvirt_dbus.syms
 create mode 100644 src/util/virdbus.c
 create mode 100644 src/util/virdbus.h

diff --git a/.gitignore b/.gitignore
index 5aa9c9b..14a21d0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -48,12 +48,12 @@
 /daemon/*_dispatch.h
 /daemon/libvirt_qemud
 /daemon/libvirtd
-/daemon/libvirtd.init
-/daemon/libvirtd.service
 /daemon/libvirtd*.logrotate
 /daemon/libvirtd.8
 /daemon/libvirtd.8.in
+/daemon/libvirtd.init
 /daemon/libvirtd.pod
+/daemon/libvirtd.service
 /docs/devhelp/libvirt.devhelp
 /docs/hvsupport.html.in
 /docs/libvirt-api.xml
@@ -118,6 +118,7 @@
 /tests/eventtest
 /tests/hashtest
 /tests/jsontest
+/tests/libvirtdconftest
 /tests/networkxml2argvtest
 /tests/nodeinfotest
 /tests/nwfilterxml2xmltest
@@ -150,7 +151,6 @@
 /tests/vmx2xmltest
 /tests/xencapstest
 /tests/xmconfigtest
-/tests/libvirtdconftest
 /tools/*.[18]
 /tools/libvirt-guests.init
 /tools/virsh
diff --git a/configure.ac b/configure.ac
index 3f5b3ff..f49b620 100644
--- a/configure.ac
+++ b/configure.ac
@@ -74,6 +74,7 @@ LIBPCAP_REQUIRED=1.0.0
 LIBNL_REQUIRED=1.1
 LIBSSH2_REQUIRED=1.0
 LIBBLKID_REQUIRED=2.17
+DBUS_REQUIRED=1.0.0
 
 dnl Checks for C compiler.
 AC_PROG_CC
@@ -1099,6 +1100,36 @@ AC_SUBST([SANLOCK_CFLAGS])
 AC_SUBST([SANLOCK_LIBS])
 
 
+dnl DBus library
+DBUS_CFLAGS=
+DBUS_LIBS=
+AC_ARG_WITH([dbus],
+  AC_HELP_STRING([--with-dbus], [enable communication with DBus 
@:@default=check@:@]),
+  [],
+  [with_dbus=check])
+if test $with_dbus = yes || test $with_dbus = check ; then
+  PKG_CHECK_MODULES(DBUS, dbus-1 = $DBUS_REQUIRED,
+[with_dbus=yes], [
+ if test $with_dbus = check ; then
+   with_dbus=no
+ else
+   AC_MSG_ERROR([You must install DBus = $DBUS_REQUIRED to compile 
libvirt])
+ fi])
+fi
+
+if test $with_dbus = yes ; then
+  AC_DEFINE_UNQUOTED([HAVE_DBUS], 1, [enable communication with DBus])
+
+  save_LIBS=$LIBS
+  save_CFLAGS=$CFLAGS
+  LIBS=$LIBS $DBUS_LIBS
+  CFLAGS=$CFLAGS $DBUS_CFLAGS
+  AC_CHECK_FUNCS([dbus_watch_get_unix_fd])
+  LIBS=$save_LIBS
+  CFLAGS=$save_CFLAGS
+fi
+
+
 dnl PolicyKit library
 POLKIT_CFLAGS=
 POLKIT_LIBS=
@@ -1109,7 +1140,6 @@ AC_ARG_WITH([polkit],
   [with_polkit=check])
 
 with_polkit0=no
-with_dbus=no
 with_polkit1=no
 if test x$with_polkit = xyes || test x$with_polkit = xcheck; then
   dnl Check for new polkit first - just a binary
@@ -1138,8 +1168,6 @@ if test x$with_polkit = xyes || test x$with_polkit 
= xcheck; then
 [use PolicyKit for UNIX socket access checks])
   AC_DEFINE_UNQUOTED([HAVE_POLKIT0], 1,
 [use PolicyKit for UNIX socket access checks])
-  AC_DEFINE_UNQUOTED([HAVE_DBUS], 1,
-[use DBus for PolicyKit])
 
   old_CFLAGS=$CFLAGS
   old_LIBS=$LIBS
@@ -1154,13 +1182,11 @@ if test x$with_polkit = xyes || test 
x$with_polkit = xcheck; then
 AC_DEFINE_UNQUOTED([POLKIT_AUTH],[$POLKIT_AUTH],[Location of 
polkit-auth program])
   fi
   with_polkit0=yes
-  with_dbus=yes
 fi
   fi
 fi
 AM_CONDITIONAL([HAVE_POLKIT], [test x$with_polkit = xyes])
 AM_CONDITIONAL([HAVE_POLKIT0], [test x$with_polkit0 = xyes])
-AM_CONDITIONAL([HAVE_DBUS], [test x$with_dbus = xyes])
 AM_CONDITIONAL([HAVE_POLKIT1], [test x$with_polkit1 = xyes])
 AC_SUBST([POLKIT_CFLAGS])
 AC_SUBST([POLKIT_LIBS])
@@ -2413,7 +2439,6 @@ if test x$with_hal = xyes || test x$with_hal = 
xcheck; then
 CFLAGS=$CFLAGS $HAL_CFLAGS
 LIBS=$LIBS $HAL_LIBS
 AC_CHECK_FUNCS([libhal_get_all_devices],,[with_hal=no])
-AC_CHECK_FUNCS([dbus_watch_get_unix_fd])
 CFLAGS=$old_CFLAGS
 LIBS=$old_LIBS
   fi
diff --git a/daemon/Makefile.am 

[libvirt] [libvirt-glib] Getter/setter for disk source's startupPolicy attribute

2012-04-12 Thread Zeeshan Ali (Khattak)
From: Zeeshan Ali (Khattak) zeesha...@gnome.org

---
 libvirt-gconfig/libvirt-gconfig-domain-disk.c |   26 +
 libvirt-gconfig/libvirt-gconfig-domain-disk.h |9 
 libvirt-gconfig/libvirt-gconfig.sym   |5 +++-
 libvirt-gconfig/tests/test-domain-create.c|2 +
 4 files changed, 41 insertions(+), 1 deletions(-)

diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.c 
b/libvirt-gconfig/libvirt-gconfig-domain-disk.c
index 5d0acb5..a29ea47 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-disk.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.c
@@ -127,6 +127,18 @@ void 
gvir_config_domain_disk_set_snapshot_type(GVirConfigDomainDisk *disk,
type, NULL);
 }
 
+void gvir_config_domain_disk_set_startup_policy(GVirConfigDomainDisk *disk,
+
GVirConfigDomainDiskStartupPolicy policy)
+{
+const char *str;
+
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk));
+str = 
gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_DISK_STARTUP_POLICY, policy);
+g_return_if_fail(str != NULL);
+gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk),
+source, startupPolicy, 
str);
+}
+
 void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk,
 const char *source)
 {
@@ -235,6 +247,19 @@ 
gvir_config_domain_disk_get_snapshot_type(GVirConfigDomainDisk *disk)
   
GVIR_CONFIG_DOMAIN_DISK_SNAPSHOT_NO);
 }
 
+GVirConfigDomainDiskStartupPolicy
+gvir_config_domain_disk_get_startup_policy(GVirConfigDomainDisk *disk)
+{
+g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk),
+ GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_MANDATORY);
+
+return gvir_config_object_get_attribute_genum
+(GVIR_CONFIG_OBJECT(disk),
+ source, startupPolicy,
+ GVIR_CONFIG_TYPE_DOMAIN_DISK_STARTUP_POLICY,
+ GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_MANDATORY);
+}
+
 const char *
 gvir_config_domain_disk_get_source(GVirConfigDomainDisk *disk)
 {
@@ -291,6 +316,7 @@ 
gvir_config_domain_disk_get_driver_cache(GVirConfigDomainDisk *disk)
   
GVIR_CONFIG_TYPE_DOMAIN_DISK_CACHE_TYPE,
   
GVIR_CONFIG_DOMAIN_DISK_CACHE_DEFAULT);
 }
+
 GVirConfigDomainDiskBus
 gvir_config_domain_disk_get_target_bus(GVirConfigDomainDisk *disk)
 {
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.h 
b/libvirt-gconfig/libvirt-gconfig-domain-disk.h
index 916421d..7e85d75 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-disk.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.h
@@ -95,6 +95,12 @@ typedef enum {
 GVIR_CONFIG_DOMAIN_DISK_SNAPSHOT_EXTERNAL
 } GVirConfigDomainDiskSnapshotType;
 
+typedef enum {
+GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_MANDATORY,
+GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_REQUISITE,
+GVIR_CONFIG_DOMAIN_DISK_STARTUP_POLICY_OPTIONAL
+} GVirConfigDomainDiskStartupPolicy;
+
 GType gvir_config_domain_disk_get_type(void);
 
 GVirConfigDomainDisk *gvir_config_domain_disk_new(void);
@@ -107,6 +113,8 @@ void 
gvir_config_domain_disk_set_guest_device_type(GVirConfigDomainDisk *disk,

GVirConfigDomainDiskGuestDeviceType type);
 void gvir_config_domain_disk_set_snapshot_type(GVirConfigDomainDisk *disk,

GVirConfigDomainDiskSnapshotType type);
+void gvir_config_domain_disk_set_startup_policy(GVirConfigDomainDisk *disk,
+
GVirConfigDomainDiskStartupPolicy policy);
 void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk,
 const char *source);
 void gvir_config_domain_disk_set_driver_cache(GVirConfigDomainDisk *disk,
@@ -123,6 +131,7 @@ void 
gvir_config_domain_disk_set_target_dev(GVirConfigDomainDisk *disk,
 GVirConfigDomainDiskType 
gvir_config_domain_disk_get_disk_type(GVirConfigDomainDisk *disk);
 GVirConfigDomainDiskGuestDeviceType 
gvir_config_domain_disk_get_guest_device_type(GVirConfigDomainDisk *disk);
 GVirConfigDomainDiskSnapshotType 
gvir_config_domain_disk_get_snapshot_type(GVirConfigDomainDisk *disk);
+GVirConfigDomainDiskStartupPolicy 
gvir_config_domain_disk_get_startup_policy(GVirConfigDomainDisk *disk);
 const char *gvir_config_domain_disk_get_source(GVirConfigDomainDisk *disk);
 GVirConfigDomainDiskCacheType 
gvir_config_domain_disk_get_driver_cache(GVirConfigDomainDisk *disk);
 const char *gvir_config_domain_disk_get_driver_name(GVirConfigDomainDisk 
*disk);
diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
b/libvirt-gconfig/libvirt-gconfig.sym
index 2378a3c..8dac83a 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ 

Re: [libvirt] [Guidelines Change] Changes to the Packaging Guidelines

2012-04-12 Thread Eric Blake
things we should be thinking about:

On 04/12/2012 02:57 PM, Tom Callaway wrote:
 Here is the latest set of changes to the Fedora Packaging Guidelines:
 
 ---
 
 Packages which have SysV initscripts that contain 'non-standard service
 commands' (commands besides start, stop, reload, restart, or
 try-restart) must convert those commands into standalone helper scripts.
 Systemd does not support non-standard unit commands.
 
 https://fedoraproject.org/wiki/Packaging:Systemd#Unit_Files

I think libvirt-guests falls into this category.

 
 ---
 
 The guidelines relating to PIE and Hardened Packages were updated. Now,
 if your package meets the following critera you MUST enable the PIE
 compiler flags:
 
 * Your package is long running. This means it's likely to be started and
 keep running until the machine is rebooted, not start on demand and quit
 on idle.
 
 * Your package has suid binaries, or binaries with capabilities.
 
 * Your package runs as root.
 
 https://fedoraproject.org/wiki/Packaging:Guidelines#PIE

libvirtd definitely qualifies as one of these packages needing PIE
compilation in our libvirt.spec file.

 
 ---
 
 Rules involving appropriate scripting within Fedora Package spec files
 were added to the Guidelines:
 
 https://fedoraproject.org/wiki/Packaging:Guidelines#Scripting_inside_of_spec_files

Don't know if any of these changes impact us, but can't hurt to audit it.

Plus, we still haven't converted our mingw specfile over to the mingw64
toolchain.  Anyone up for some specfile maintenance?

-- 
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] xend_internal: Use domain/status for shutdown check

2012-04-12 Thread Cole Robinson
On 04/12/2012 09:42 AM, Stefan Bader wrote:
 As promised this version does keep the domid  0 check in
 order to be clearly keeping the old behavior.
 
 -Stefan
 
From 18d398d98dc0dc2d9148ffb8673c651248d1bca5 Mon Sep 17 00:00:00 2001
 From: Stefan Bader stefan.ba...@canonical.com
 Date: Thu, 12 Apr 2012 09:59:56 +
 Subject: [PATCH] xend_internal: Use domain/status for shutdown check
 
 On newer xend (v3.x and after) there is no state and domid reported
 for inactive domains. When initially creating connections this is
 handled in various places by assigning domain-id = -1.
 But once an instance has been running, the id is set to the current
 domain id. And it does not change when the instance is shut down.
 So when querying the domain info, the hypervisor driver, which gets
 asked first will indicate it cannot find information, then the
 xend driver is asked and will set the status to NOSTATE because it
 checks for the -1 domain id.
 Checking domain/status for 0 seems to be more reliable for that.
 
 One note: I am not sure whether the domain-id also should get set
 back to -1 whenever any sub-driver thinks the instance is no longer
 running.
 
 BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=746007
 BugLink: http://bugs.launchpad.net/bugs/929626
 
 [v2: Keep old id check just in case]
 Signed-off-by: Stefan Bader stefan.ba...@canonical.com
 ---
  src/xen/xend_internal.c |   11 ---
  1 file changed, 8 insertions(+), 3 deletions(-)
 
 diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
 index 6526af4..f1aa9b6 100644
 --- a/src/xen/xend_internal.c
 +++ b/src/xen/xend_internal.c
 @@ -989,9 +989,14 @@ sexpr_to_xend_domain_state(virDomainPtr domain, const 
 struct sexpr *root)
  state = VIR_DOMAIN_BLOCKED;
  else if (strchr(flags, 'r'))
  state = VIR_DOMAIN_RUNNING;
 -} else if (domain-id  0) {
 -/* Inactive domains don't have a state reported, so
 -   mark them SHUTOFF, rather than NOSTATE */
 +} else if (domain-id  0 || sexpr_int(root, domain/status) == 0) {
 +/* As far as I can see the domain-id is a bad sign for checking
 + * inactive domains as this is inaccurate after the domain has
 + * been running once. However domain/status from xend seems to
 + * be always present and 0 for inactive domains.
 + * (keeping the check for id  0 to be extra safe about backward
 + * compatibility)
 + */
  state = VIR_DOMAIN_SHUTOFF;
  }
  

ACK

Thanks,
Cole

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


Re: [libvirt] [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC

2012-04-12 Thread Cole Robinson
On 04/12/2012 09:42 AM, Stefan Bader wrote:
 This is a re-send as there was some positive feedback but the
 patch itself made it into the repo.
 
 -Stefan
 
From a3198c5c1ae8908818f6c0f0df4237dbe5ddeec7 Mon Sep 17 00:00:00 2001
 From: Stefan Bader stefan.ba...@canonical.com
 Date: Thu, 12 Apr 2012 15:32:41 +0200
 Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC
 
 When using the xm/xend stack to manage instances there is a bug
 that causes the emulated interfaces to be unusable when the vif
 config contains type=ioemu (MAC address all zero).
 
 The current code already has a special quirk to not use this
 keyword if no specific model is given for the emulated NIC
 (defaulting to rtl8139).
 Essentially it works because regardless of the type argument,
 the Xen stack always creates emulated and paravirt interfaces and
 lets the guest decide which one to use. So neither xl nor xm stack
 actually require the type keyword for emulated NICs.
 
 Signed-off-by: Stefan Bader stefan.ba...@canonical.com
 ---
  src/xenxs/xen_sxpr.c |4 +++-
  src/xenxs/xen_xm.c   |4 +++-
  2 files changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
 index e1bbd76..71602fa 100644
 --- a/src/xenxs/xen_sxpr.c
 +++ b/src/xenxs/xen_sxpr.c
 @@ -2012,7 +2012,9 @@ xenFormatSxprNet(virConnectPtr conn,
  }
  else {
  virBufferEscapeSexpr(buf, (model '%s'), def-model);
 -virBufferAddLit(buf, (type ioemu));
 + /* See above. Also needed when model is specified. */
 +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
 +virBufferAddLit(buf, (type ioemu));
  }
  

I think it would be better if we just centralized this logic, as in, only set
that (type ioemu) bit in conditional rather than 2. Should be pretty
straightforward.

  if (!isAttach)
 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index d65e97a..e072599 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -1394,7 +1394,9 @@ static int xenFormatXMNet(virConnectPtr conn,
  }
  else {
  virBufferAsprintf(buf, ,model=%s, net-model);
 -virBufferAddLit(buf, ,type=ioemu);
 + /* See above. Also needed if model is specified. */
 +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
 +virBufferAddLit(buf, ,type=ioemu);
  }
  
  if (net-ifname)

Same here as well.

Thanks,
Cole

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


Re: [libvirt] [PATCH] xend_internal: Use domain/status for shutdown check

2012-04-12 Thread Eric Blake
On 04/12/2012 03:50 PM, Cole Robinson wrote:
 On 04/12/2012 09:42 AM, Stefan Bader wrote:
 As promised this version does keep the domid  0 check in
 order to be clearly keeping the old behavior.

 -Stefan

 From 18d398d98dc0dc2d9148ffb8673c651248d1bca5 Mon Sep 17 00:00:00 2001
 From: Stefan Bader stefan.ba...@canonical.com
 Date: Thu, 12 Apr 2012 09:59:56 +
 Subject: [PATCH] xend_internal: Use domain/status for shutdown check

 On newer xend (v3.x and after) there is no state and domid reported
 for inactive domains. When initially creating connections this is
 handled in various places by assigning domain-id = -1.

 
 ACK

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 v2] Pull DBus event code out into common area

2012-04-12 Thread Eric Blake
On 04/12/2012 02:07 PM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The policy kit and HAL node device drivers both require a
 DBus connection. The HAL device code further requires that
 the DBus connection is integrated with the event loop and
 provides such glue logic itself.
 
 The forthcoming FirewallD integration also requires a
 dbus connection with event loop integration. Thus we need
 to pull the current event loop glue out of the HAL driver.
 
 Thus we create src/util/virdbus.{c,h} files. This contains
 just one method virDBusGetSystemBus() which obtains a handle
 to the single shared system bus instance, with event glue
 automagically setup.
 ---
  .gitignore|6 +-
  configure.ac  |   37 ++-
  daemon/Makefile.am|3 +-
  daemon/libvirtd.c |4 -
  daemon/remote.c   |8 +-
  include/libvirt/virterror.h   |1 +
  src/Makefile.am   |   13 +--
  src/libvirt_dbus.syms |2 -

You're dropping libvirt_dbus.syms, but I don't see where you are
modifying libvirt_private.syms to make up for it.

  src/node_device/node_device_hal.c |  143 ++
  src/rpc/virnetserver.c|   40 
  src/rpc/virnetserver.h|8 --
  src/util/virdbus.c|  201 
 +
  src/util/virdbus.h|   34 ++
  src/util/virterror.c  |3 +
  14 files changed, 296 insertions(+), 207 deletions(-)
  delete mode 100644 src/libvirt_dbus.syms
  create mode 100644 src/util/virdbus.c
  create mode 100644 src/util/virdbus.h


 +#ifndef __VIR_DBUS_H__
 +# define __VIR_DBUS_H__
 +
 +# ifdef HAVE_DBUS
 +#  include dbus/dbus.h
 +# else
 +#  define DBusConnection void

Is typedef any better than #define here?  But this works.

ACK with .syms nit fixed.

-- 
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