[libvirt] [libvirt-designer 0/7]

Hey, After seeing the libvirt-designer GSoC idea, I remembered about a bunch of libvirt-designer patches I had never sent :) Here they are, a few leak fixes/build cleanups, and addition of 2 new methods for completeness. Christophe

The latter is deprecated in favour of the former. --- examples/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/Makefile.am b/examples/Makefile.am index 1d8947f..65a5afe 100644 --- a/examples/Makefile.am +++ b/examples/Makefile.am @@ -1,4 +1,4 @@ -INCLUDES = \ +AM_CPPFLAGS = \ -I$(top_builddir)/libvirt-designer \ -I$(top_srcdir) -- 2.1.0

This fixes gcc warning about -Wmudflap on fedora 21 --- m4/manywarnings.m4 | 213 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 137 insertions(+), 76 deletions(-) diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4 index fd0e372..3e6dd21 100644 --- a/m4/manywarnings.m4 +++ b/m4/manywarnings.m4 @@ -1,5 +1,5 @@ -# manywarnings.m4 serial 3 -dnl Copyright (C) 2008-2012 Free Software Foundation, Inc. +# manywarnings.m4 serial 7 +dnl Copyright (C) 2008-2014 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, dnl with or without modifications, as long as this notice is preserved. @@ -35,14 +35,12 @@ AC_DEFUN([gl_MANYWARN_COMPLEMENT], # make sure your gcc understands it. AC_DEFUN([gl_MANYWARN_ALL_GCC], [ - dnl First, check if -Wno-missing-field-initializers is needed. - dnl -Wmissing-field-initializers is implied by -W, but that issues - dnl warnings with GCC version before 4.7, for the common idiom - dnl of initializing types on the stack to zero, using { 0, } + dnl First, check for some issues that only occur when combining multiple + dnl gcc warning categories. AC_REQUIRE([AC_PROG_CC]) if test -n "$GCC"; then - dnl First, check -W -Werror -Wno-missing-field-initializers is supported + dnl Check if -W -Werror -Wno-missing-field-initializers is supported dnl with the current $CC $CFLAGS $CPPFLAGS. AC_MSG_CHECKING([whether -Wno-missing-field-initializers is supported]) AC_CACHE_VAL([gl_cv_cc_nomfi_supported], [ @@ -77,108 +75,171 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC], ]) AC_MSG_RESULT([$gl_cv_cc_nomfi_needed]) fi + + dnl Next, check if -Werror -Wuninitialized is useful with the + dnl user's choice of $CFLAGS; some versions of gcc warn that it + dnl has no effect if -O is not also used + AC_MSG_CHECKING([whether -Wuninitialized is supported]) + AC_CACHE_VAL([gl_cv_cc_uninitialized_supported], [ + gl_save_CFLAGS="$CFLAGS" + CFLAGS="$CFLAGS -Werror -Wuninitialized" + AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([[]], [[]])], + [gl_cv_cc_uninitialized_supported=yes], + [gl_cv_cc_uninitialized_supported=no]) + CFLAGS="$gl_save_CFLAGS"]) + AC_MSG_RESULT([$gl_cv_cc_uninitialized_supported]) + fi + # List all gcc warning categories. + # To compare this list to your installed GCC's, run this Bash command: + # + # comm -3 \ + # <(sed -n 's/^ *\(-[^ ]*\) .*/\1/p' manywarnings.m4 | sort) \ + # <(gcc --help=warnings | sed -n 's/^ \(-[^ ]*\) .*/\1/p' | sort | + # grep -v -x -f <( + # awk '/^[^#]/ {print $1}' ../build-aux/gcc-warning.spec)) + gl_manywarn_set= for gl_manywarn_item in \ - -Wall \ -W \ - -Wformat-y2k \ - -Wformat-nonliteral \ - -Wformat-security \ - -Winit-self \ - -Wmissing-include-dirs \ - -Wswitch-default \ - -Wswitch-enum \ - -Wunused \ - -Wunknown-pragmas \ - -Wstrict-aliasing \ - -Wstrict-overflow \ - -Wsystem-headers \ - -Wfloat-equal \ - -Wtraditional \ - -Wtraditional-conversion \ - -Wdeclaration-after-statement \ - -Wundef \ - -Wshadow \ - -Wunsafe-loop-optimizations \ - -Wpointer-arith \ + -Wabi \ + -Waddress \ + -Waggressive-loop-optimizations \ + -Wall \ + -Warray-bounds \ + -Wattributes \ -Wbad-function-cast \ - -Wc++-compat \ - -Wcast-qual \ - -Wcast-align \ - -Wwrite-strings \ - -Wconversion \ - -Wsign-conversion \ - -Wlogical-op \ - -Waggregate-return \ - -Wstrict-prototypes \ - -Wold-style-definition \ - -Wmissing-prototypes \ - -Wmissing-declarations \ - -Wmissing-noreturn \ - -Wmissing-format-attribute \ - -Wpacked \ - -Wpadded \ - -Wredundant-decls \ - -Wnested-externs \ - -Wunreachable-code \ - -Winline \ - -Winvalid-pch \ - -Wlong-long \ - -Wvla \ - -Wvolatile-register-var \ - -Wdisabled-optimization \ - -Wstack-protector \ - -Woverlength-strings \ -Wbuiltin-macro-redefined \ - -Wmudflap \ - -Wpacked-bitfield-compat \ - -Wsync-nand \ - ; do - gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item" - done - # The following are not documented in the manual but are included in - # output from gcc --help=warnings. - for gl_manywarn_item in \ - -Wattributes \ + -Wcast-align \ + -Wchar-subscripts \ + -Wclobbered \ + -Wcomment \ + -Wcomments \ -Wcoverage-mismatch \ - -Wmultichar \ - -Wunused-macros \ - ; do - gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item" - done - # More warnings from gcc 4.6.2 --help=warnings. - for gl_manywarn_item in \ - -Wabi \ -Wcpp \ + -Wdate-time \ -Wdeprecated \ -Wdeprecated-declarations \ + -Wdisabled-optimization \ -Wdiv-by-zero \ -Wdouble-promotion \ + -Wempty-body \ -Wendif-labels \ + -Wenum-compare \ -Wextra \ -Wformat-contains-nul \ -Wformat-extra-args \ + -Wformat-nonliteral \ + -Wformat-security \ + -Wformat-y2k \ -Wformat-zero-length \ - -Wformat=2 \ + -Wfree-nonheap-object \ + -Wignored-qualifiers \ + -Wimplicit \ + -Wimplicit-function-declaration \ + -Wimplicit-int \ + -Winit-self \ + -Winline \ + -Wint-to-pointer-cast \ + -Winvalid-memory-model \ + -Winvalid-pch \ + -Wjump-misses-init \ + -Wlogical-op \ + -Wmain \ + -Wmaybe-uninitialized \ + -Wmissing-braces \ + -Wmissing-declarations \ + -Wmissing-field-initializers \ + -Wmissing-include-dirs \ + -Wmissing-parameter-type \ + -Wmissing-prototypes \ -Wmultichar \ - -Wnormalized=nfc \ + -Wnarrowing \ + -Wnested-externs \ + -Wnonnull \ + -Wold-style-declaration \ + -Wold-style-definition \ + -Wopenmp-simd \ -Woverflow \ + -Woverlength-strings \ + -Woverride-init \ + -Wpacked \ + -Wpacked-bitfield-compat \ + -Wparentheses \ + -Wpointer-arith \ + -Wpointer-sign \ -Wpointer-to-int-cast \ -Wpragmas \ + -Wreturn-local-addr \ + -Wreturn-type \ + -Wsequence-point \ + -Wshadow \ + -Wsizeof-pointer-memaccess \ + -Wstack-protector \ + -Wstrict-aliasing \ + -Wstrict-overflow \ + -Wstrict-prototypes \ -Wsuggest-attribute=const \ + -Wsuggest-attribute=format \ -Wsuggest-attribute=noreturn \ -Wsuggest-attribute=pure \ + -Wswitch \ + -Wswitch-default \ + -Wsync-nand \ + -Wsystem-headers \ -Wtrampolines \ + -Wtrigraphs \ + -Wtype-limits \ + -Wuninitialized \ + -Wunknown-pragmas \ + -Wunsafe-loop-optimizations \ + -Wunused \ + -Wunused-but-set-parameter \ + -Wunused-but-set-variable \ + -Wunused-function \ + -Wunused-label \ + -Wunused-local-typedefs \ + -Wunused-macros \ + -Wunused-parameter \ + -Wunused-result \ + -Wunused-value \ + -Wunused-variable \ + -Wvarargs \ + -Wvariadic-macros \ + -Wvector-operation-performance \ + -Wvla \ + -Wvolatile-register-var \ + -Wwrite-strings \ + \ ; do gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item" done - # Disable the missing-field-initializers warning if needed + # gcc --help=warnings outputs an unusual form for this option; list + # it here so that the above 'comm' command doesn't report a false match. + gl_manywarn_set="$gl_manywarn_set -Wnormalized=nfc" + + # These are needed for older GCC versions. + if test -n "$GCC"; then + case `($CC --version) 2>/dev/null` in + 'gcc (GCC) '[[0-3]].* | \ + 'gcc (GCC) '4.[[0-7]].*) + gl_manywarn_set="$gl_manywarn_set -fdiagnostics-show-option" + gl_manywarn_set="$gl_manywarn_set -funit-at-a-time" + ;; + esac + fi + + # Disable specific options as needed. if test "$gl_cv_cc_nomfi_needed" = yes; then gl_manywarn_set="$gl_manywarn_set -Wno-missing-field-initializers" fi + if test "$gl_cv_cc_uninitialized_supported" = no; then + gl_manywarn_set="$gl_manywarn_set -Wno-uninitialized" + fi + $1=$gl_manywarn_set ]) -- 2.1.0

The caller of gvir_designer_domain_add_interface_network() owns a reference on the returned GVirConfigDomainInterface instance, so it needs to be released when no longer needed. --- examples/virt-designer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/virt-designer.c b/examples/virt-designer.c index 7628449..f134b98 100644 --- a/examples/virt-designer.c +++ b/examples/virt-designer.c @@ -350,6 +350,7 @@ add_iface(gpointer data, exit(EXIT_FAILURE); } } + g_object_unref(iface); } static gboolean -- 2.1.0

When an unknown NIC type is passed to gvir_designer_domain_add_interface_full(), 'ret' would be returned uninitialized to the caller. --- libvirt-designer/libvirt-designer-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 5475c79..7387ab1 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -1872,7 +1872,7 @@ gvir_designer_domain_add_interface_full(GVirDesignerDomain *design, const char *network, GError **error) { - GVirConfigDomainInterface *ret; + GVirConfigDomainInterface *ret = NULL; const gchar *model = NULL; model = gvir_designer_domain_get_preferred_nic_model(design, error); -- 2.1.0

--- libvirt-designer/libvirt-designer-domain.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 7387ab1..0321180 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -1916,6 +1916,8 @@ gvir_designer_domain_add_interface_network(GVirDesignerDomain *design, GError **error) { g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL); + g_return_val_if_fail(network != NULL, NULL); + g_return_val_if_fail(!error_is_set(error), NULL); GVirConfigDomainInterface *ret = NULL; -- 2.1.0

These cover 2 additional types of libvirt interfaces: usermode/SLIRP networking and bridge. --- libvirt-designer/libvirt-designer-domain.c | 70 +++++++++++++++++++++++++++++- libvirt-designer/libvirt-designer-domain.h | 5 +++ libvirt-designer/libvirt-designer.sym | 2 + 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 0321180..a060a81 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -54,7 +54,9 @@ G_DEFINE_TYPE(GVirDesignerDomain, gvir_designer_domain, G_TYPE_OBJECT); #define GVIR_DESIGNER_DOMAIN_ERROR gvir_designer_domain_error_quark() typedef enum { + GVIR_DESIGNER_DOMAIN_NIC_TYPE_BRIDGE, GVIR_DESIGNER_DOMAIN_NIC_TYPE_NETWORK, + GVIR_DESIGNER_DOMAIN_NIC_TYPE_USER, /* add new type here */ } GVirDesignerDomainNICType; @@ -1869,7 +1871,7 @@ cleanup: static GVirConfigDomainInterface * gvir_designer_domain_add_interface_full(GVirDesignerDomain *design, GVirDesignerDomainNICType type, - const char *network, + const char *source, GError **error) { GVirConfigDomainInterface *ret = NULL; @@ -1878,10 +1880,18 @@ gvir_designer_domain_add_interface_full(GVirDesignerDomain *design, model = gvir_designer_domain_get_preferred_nic_model(design, error); switch (type) { + case GVIR_DESIGNER_DOMAIN_NIC_TYPE_BRIDGE: + ret = GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_bridge_new()); + gvir_config_domain_interface_bridge_set_source(GVIR_CONFIG_DOMAIN_INTERFACE_BRIDGE(ret), + source); + break; case GVIR_DESIGNER_DOMAIN_NIC_TYPE_NETWORK: ret = GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_network_new()); gvir_config_domain_interface_network_set_source(GVIR_CONFIG_DOMAIN_INTERFACE_NETWORK(ret), - network); + source); + break; + case GVIR_DESIGNER_DOMAIN_NIC_TYPE_USER: + ret = GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_user_new()); break; default: g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0, @@ -1898,6 +1908,35 @@ cleanup: return ret; } +/** + * gvir_designer_domain_add_interface_bridge: + * @design: (transfer none): the domain designer instance + * @bridge: (transfer none): bridge name + * @error: return location for a #GError, or NULL + * + * Add new network interface card into @design. The interface is + * of 'bridge' type and uses @bridge as the bridge device + * + * Returns: (transfer full): the pointer to the new interface. + */ +GVirConfigDomainInterface * +gvir_designer_domain_add_interface_bridge(GVirDesignerDomain *design, + const char *bridge, + GError **error) +{ + g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL); + g_return_val_if_fail(bridge != NULL, NULL); + g_return_val_if_fail(!error_is_set(error), NULL); + + GVirConfigDomainInterface *ret = NULL; + + ret = gvir_designer_domain_add_interface_full(design, + GVIR_DESIGNER_DOMAIN_NIC_TYPE_BRIDGE, + bridge, + error); + + return ret; +} /** * gvir_designer_domain_add_interface_network: @@ -1929,6 +1968,33 @@ gvir_designer_domain_add_interface_network(GVirDesignerDomain *design, return ret; } +/** + * gvir_designer_domain_add_interface_user: + * @design: (transfer none): the domain designer instance + * @error: return location for a #GError, or NULL + * + * Add new network interface card into @design. The interface is + * of 'user' type. + * + * Returns: (transfer full): the pointer to the new interface. + */ +GVirConfigDomainInterface * +gvir_designer_domain_add_interface_user(GVirDesignerDomain *design, + GError **error) +{ + g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL); + g_return_val_if_fail(!error_is_set(error), NULL); + + GVirConfigDomainInterface *ret = NULL; + + ret = gvir_designer_domain_add_interface_full(design, + GVIR_DESIGNER_DOMAIN_NIC_TYPE_USER, + NULL, + error); + + return ret; +} + static GVirConfigDomainVideoModel gvir_designer_domain_video_model_str_to_enum(const char *model_str, GError **error) diff --git a/libvirt-designer/libvirt-designer-domain.h b/libvirt-designer/libvirt-designer-domain.h index 85dce97..41aae66 100644 --- a/libvirt-designer/libvirt-designer-domain.h +++ b/libvirt-designer/libvirt-designer-domain.h @@ -128,9 +128,14 @@ GVirConfigDomainDisk *gvir_designer_domain_add_floppy_device(GVirDesignerDomain const char *devpath, GError **error); +GVirConfigDomainInterface *gvir_designer_domain_add_interface_bridge(GVirDesignerDomain *design, + const char *bridge, + GError **error); GVirConfigDomainInterface *gvir_designer_domain_add_interface_network(GVirDesignerDomain *design, const char *network, GError **error); +GVirConfigDomainInterface *gvir_designer_domain_add_interface_user(GVirDesignerDomain *design, + GError **error); GVirConfigDomainGraphics *gvir_designer_domain_add_graphics(GVirDesignerDomain *design, GVirDesignerDomainGraphics type, diff --git a/libvirt-designer/libvirt-designer.sym b/libvirt-designer/libvirt-designer.sym index 96bc656..d5a2e6e 100644 --- a/libvirt-designer/libvirt-designer.sym +++ b/libvirt-designer/libvirt-designer.sym @@ -21,7 +21,9 @@ LIBVIRT_DESIGNER_0.0.2 { gvir_designer_domain_add_floppy_file; gvir_designer_domain_add_floppy_device; gvir_designer_domain_add_graphics; + gvir_designer_domain_add_interface_bridge; gvir_designer_domain_add_interface_network; + gvir_designer_domain_add_interface_user; gvir_designer_domain_add_smartcard; gvir_designer_domain_add_sound; gvir_designer_domain_add_usb_redir; -- 2.1.0

A GVirConfigDomainDiskDriver is created in gvir_designer_domain_add_disk_full but it's never unref'ed. This commit fixes that, including when an error occurs. --- libvirt-designer/libvirt-designer-domain.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index a060a81..c1a0e83 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -1626,6 +1626,8 @@ gvir_designer_domain_add_disk_full(GVirDesignerDomain *design, gvir_config_domain_disk_set_type(disk, type); gvir_config_domain_disk_set_source(disk, path); gvir_config_domain_disk_set_driver(disk, driver); + g_object_unref(driver); + driver = NULL; controller = gvir_designer_domain_get_preferred_disk_controller(design, NULL); if (controller == NULL) @@ -1656,6 +1658,8 @@ gvir_designer_domain_add_disk_full(GVirDesignerDomain *design, error: g_free(target_gen); + if (driver != NULL) + g_object_unref(driver); if (disk) g_object_unref(disk); return NULL; -- 2.1.0

On 03.03.2015 12:11, Christophe Fergeau wrote:
Hey,
After seeing the libvirt-designer GSoC idea, I remembered about a bunch of libvirt-designer patches I had never sent :) Here they are, a few leak fixes/build cleanups, and addition of 2 new methods for completeness.
Christophe
ACK series. Michal

On Wed, Mar 04, 2015 at 10:32:34AM +0100, Michal Privoznik wrote:
On 03.03.2015 12:11, Christophe Fergeau wrote:
Hey,
After seeing the libvirt-designer GSoC idea, I remembered about a bunch of libvirt-designer patches I had never sent :) Here they are, a few leak fixes/build cleanups, and addition of 2 new methods for completeness.
Christophe
ACK series.
Thanks, pushed. Christophe
participants (2)
-
Christophe Fergeau
-
Michal Privoznik