[libvirt] [PATCH] daemon: Plug memory leaks
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
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
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
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
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
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
* 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
-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
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
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
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
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
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
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
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
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
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
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
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
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
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