[libvirt] [libvirt-designer][PATCH 0/4] Basic disk & interface support
with small example called virtxml Michal Privoznik (4): Load osinfo DB on init domain: Introduce disk support examples: Create an example of usage program domain: Introduce interface support .gitignore | 1 + Makefile.am | 2 +- configure.ac | 6 +- examples/Makefile.am | 23 ++ examples/virtxml.c | 406 ++++++++++++++++++++++++++ libvirt-designer/Makefile.am | 1 + libvirt-designer/libvirt-designer-domain.c | 302 +++++++++++++++++++- libvirt-designer/libvirt-designer-domain.h | 12 +- libvirt-designer/libvirt-designer-internal.h | 30 ++ libvirt-designer/libvirt-designer-main.c | 17 +- libvirt-designer/libvirt-designer.sym | 4 + 11 files changed, 799 insertions(+), 5 deletions(-) create mode 100644 examples/Makefile.am create mode 100644 examples/virtxml.c create mode 100644 libvirt-designer/libvirt-designer-internal.h -- 1.7.8.6
as we need this DB later to find an OS or hypervisor and supported devices. --- libvirt-designer/Makefile.am | 1 + libvirt-designer/libvirt-designer-domain.c | 5 +++- libvirt-designer/libvirt-designer-internal.h | 30 ++++++++++++++++++++++++++ libvirt-designer/libvirt-designer-main.c | 17 +++++++++++++- 4 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 libvirt-designer/libvirt-designer-internal.h diff --git a/libvirt-designer/Makefile.am b/libvirt-designer/Makefile.am index cf40419..8f10c41 100644 --- a/libvirt-designer/Makefile.am +++ b/libvirt-designer/Makefile.am @@ -20,6 +20,7 @@ DESIGNER_GENERATED_FILES = \ DESIGNER_HEADER_FILES = \ libvirt-designer.h \ + libvirt-designer-internal.h \ libvirt-designer-main.h \ libvirt-designer-domain.h \ $(NULL) diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 9b4a7ed..a8cabde 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -17,13 +17,16 @@ * License along with this library; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * - * Author: Daniel P. Berrange <berrange@redhat.com> + * Authors: + * Daniel P. Berrange <berrange@redhat.com> + * Michal Privoznik <mprivozn@redhat.com> */ #include <config.h> #include <sys/utsname.h> #include "libvirt-designer/libvirt-designer.h" +#include "libvirt-designer/libvirt-designer-internal.h" #define GVIR_DESIGNER_DOMAIN_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_DESIGNER_TYPE_DOMAIN, GVirDesignerDomainPrivate)) diff --git a/libvirt-designer/libvirt-designer-internal.h b/libvirt-designer/libvirt-designer-internal.h new file mode 100644 index 0000000..bbef922 --- /dev/null +++ b/libvirt-designer/libvirt-designer-internal.h @@ -0,0 +1,30 @@ +/* + * libvirt-designer-internal.h: internal definitions just + * used by code from the library + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#ifndef __LIBVIRT_DESIGNER_INTERNAL_H__ +#define __LIBVIRT_DESIGNER_INTERNAL_H__ + +extern OsinfoLoader *osinfo_loader; +extern OsinfoDb *osinfo_db; + +#endif /* __LIBVIRT_DESIGNER_INTERNAL_H__ */ diff --git a/libvirt-designer/libvirt-designer-main.c b/libvirt-designer/libvirt-designer-main.c index 60bf8f5..f2381a6 100644 --- a/libvirt-designer/libvirt-designer-main.c +++ b/libvirt-designer/libvirt-designer-main.c @@ -17,7 +17,9 @@ * License along with this library; If not, see * <http://www.gnu.org/licenses/>. * - * Author: Daniel P. Berrange <berrange@redhat.com> + * Authors: + * Daniel P. Berrange <berrange@redhat.com> + * Michal Privoznik <mprivozn@redhat.com> */ #include <config.h> @@ -28,6 +30,9 @@ #include <libvirt-designer/libvirt-designer.h> #include <libvirt-gconfig/libvirt-gconfig.h> +OsinfoLoader *osinfo_loader = NULL; +OsinfoDb *osinfo_db = NULL; + /** * gvir_designer_init: * @argc: (inout): pointer to application's argc @@ -80,5 +85,15 @@ gboolean gvir_designer_init_check(int *argc, gvir_log_handler, NULL); #endif + /* Init libosinfo and load databases from default paths */ + /* XXX maybe we want to let users tell a different path via + * env variable or argv */ + osinfo_loader = osinfo_loader_new(); + osinfo_loader_process_default_path(osinfo_loader, err); + if (err) + return FALSE; + + osinfo_db = osinfo_loader_get_db(osinfo_loader); + return TRUE; } -- 1.7.8.6
On 09/05/2012 11:27 AM, Michal Privoznik wrote:
as we need this DB later to find an OS or hypervisor and supported devices. --- libvirt-designer/Makefile.am | 1 + libvirt-designer/libvirt-designer-domain.c | 5 +++- libvirt-designer/libvirt-designer-internal.h | 30 ++++++++++++++++++++++++++ libvirt-designer/libvirt-designer-main.c | 17 +++++++++++++- 4 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 libvirt-designer/libvirt-designer-internal.h
Even though I'm not an expert for this g{lib,object}/libosinfo, I'll go ahead and try to have a look at these patches. This first one seems fine to me, so in case it means anything, ACK for this one. Martin
Let users add either files or devices as disks to domains. --- libvirt-designer/libvirt-designer-domain.c | 244 ++++++++++++++++++++++++++++ libvirt-designer/libvirt-designer-domain.h | 7 + libvirt-designer/libvirt-designer.sym | 3 + 3 files changed, 254 insertions(+), 0 deletions(-) diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index a8cabde..20611f2 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -37,6 +37,12 @@ struct _GVirDesignerDomainPrivate GVirConfigCapabilities *caps; OsinfoOs *os; OsinfoPlatform *platform; + + OsinfoDeployment *deployment; + /* next disk targets */ + unsigned int ide; + unsigned int virtio; + unsigned int sata; }; G_DEFINE_TYPE(GVirDesignerDomain, gvir_designer_domain, G_TYPE_OBJECT); @@ -134,6 +140,7 @@ static void gvir_designer_domain_finalize(GObject *object) g_object_unref(priv->os); g_object_unref(priv->platform); g_object_unref(priv->caps); + g_object_unref(priv->deployment); G_OBJECT_CLASS(gvir_designer_domain_parent_class)->finalize(object); } @@ -663,3 +670,240 @@ cleanup: g_object_unref(guest); return ret; } + +static GList * +gvir_designer_domain_get_supported_disk_bus_types(GVirDesignerDomain *design) +{ + GVirDesignerDomainPrivate *priv = design->priv; + OsinfoDeviceList *dev_list; + GHashTable *bus_hash = g_hash_table_new(g_str_hash, g_str_equal); + GList *ret = NULL; + int i; + + dev_list = osinfo_os_get_devices_by_property(priv->os, "class", "block", TRUE); + + for (i = 0; i < osinfo_list_get_length(OSINFO_LIST(dev_list)); i++) { + OsinfoDevice *dev = OSINFO_DEVICE(osinfo_list_get_nth(OSINFO_LIST(dev_list), i)); + const gchar *bus = osinfo_device_get_bus_type(dev); + + if (bus) + g_hash_table_add(bus_hash, g_strdup(bus)); + } + + ret = g_hash_table_get_keys(bus_hash); + ret = g_list_copy(ret); + + g_hash_table_destroy(bus_hash); + return ret; +} + +static OsinfoDeviceLink * +gvir_designer_domain_get_preferred_device(GVirDesignerDomain *design, + const char *class, + GError **error) +{ + GVirDesignerDomainPrivate *priv = design->priv; + OsinfoFilter *filter = osinfo_filter_new(); + OsinfoDeviceLinkFilter *filter_link = NULL; + OsinfoDeployment *deployment = priv->deployment; + OsinfoDeviceLink *dev_link = NULL; + + if (!deployment) { + priv->deployment = deployment = osinfo_db_find_deployment(osinfo_db, + priv->os, + priv->platform); + if (!deployment) { + g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0, + "Unable to find any deployment in libosinfo database"); + goto cleanup; + } + } + + osinfo_filter_add_constraint(filter, "class", class); + filter_link = osinfo_devicelinkfilter_new(filter); + dev_link = osinfo_deployment_get_preferred_device_link(deployment, OSINFO_FILTER(filter_link)); + +cleanup: + if (filter_link) + g_object_unref(filter_link); + if (filter) + g_object_unref(filter); + return dev_link; +} + +static const gchar * +gvir_designer_domain_get_preferred_disk_bus_type(GVirDesignerDomain *design, + GError **error) +{ + OsinfoDevice *dev; + OsinfoDeviceLink *dev_link = gvir_designer_domain_get_preferred_device(design, + "block", + error); + const gchar *ret = NULL; + + if (!dev_link) + return NULL; + + dev = osinfo_devicelink_get_target(dev_link); + ret = osinfo_device_get_bus_type(dev); + + return ret; +} + +static gchar * +gvir_designer_domain_next_disk_target(GVirDesignerDomain *design, + GVirConfigDomainDiskBus bus) +{ + gchar *ret = NULL; + GVirDesignerDomainPrivate *priv = design->priv; + + switch (bus) { + case GVIR_CONFIG_DOMAIN_DISK_BUS_IDE: + ret = g_strdup_printf("hd%c", 'a' + priv->ide++); + break; + case GVIR_CONFIG_DOMAIN_DISK_BUS_VIRTIO: + ret = g_strdup_printf("vd%c", 'a' + priv->virtio++); + break; + case GVIR_CONFIG_DOMAIN_DISK_BUS_SATA: + ret = g_strdup_printf("sd%c", 'a' + priv->sata++); + break; + case GVIR_CONFIG_DOMAIN_DISK_BUS_FDC: + case GVIR_CONFIG_DOMAIN_DISK_BUS_SCSI: + case GVIR_CONFIG_DOMAIN_DISK_BUS_XEN: + case GVIR_CONFIG_DOMAIN_DISK_BUS_USB: + case GVIR_CONFIG_DOMAIN_DISK_BUS_UML: + default: + /* not supported yet */ + /* XXX should we fallback to ide/virtio? */ + break; + } + + return ret; +} + +static GVirConfigDomainDisk * +gvir_designer_domain_add_disk_full(GVirDesignerDomain *design, + GVirConfigDomainDiskType type, + const char *path, + const char *format, + gchar *target, + GError **error) +{ + GVirDesignerDomainPrivate *priv = design->priv; + GVirConfigDomainDisk *disk = NULL; + GVirConfigDomainDiskBus bus; + gchar *target_gen = NULL; + const gchar *bus_str = NULL; + GList *bus_str_list = NULL, *item = NULL; + + /* Guess preferred disk bus */ + bus_str = gvir_designer_domain_get_preferred_disk_bus_type(design, error); + if (!bus_str) { + /* And fallback if fails */ + bus_str_list = gvir_designer_domain_get_supported_disk_bus_types(design); + if (!bus_str_list) + goto error; + + item = g_list_first(bus_str_list); + bus_str_list = item->data; + } + + + disk = gvir_config_domain_disk_new(); + gvir_config_domain_disk_set_type(disk, type); + gvir_config_domain_disk_set_source(disk, path); + gvir_config_domain_disk_set_driver_name(disk, "qemu"); + gvir_config_domain_disk_set_driver_type(disk, format); + if (g_str_equal(bus_str, "ide")) { + bus = GVIR_CONFIG_DOMAIN_DISK_BUS_IDE; + } else if (g_str_equal(bus_str, "virtio")) { + bus = GVIR_CONFIG_DOMAIN_DISK_BUS_VIRTIO; + } else if (g_str_equal(bus_str, "sata")) { + bus = GVIR_CONFIG_DOMAIN_DISK_BUS_SATA; + } else { + g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0, + "unsupported disk bus type '%s'", bus_str); + goto error; + } + + gvir_config_domain_disk_set_target_bus(disk, bus); + + if (!target) { + target = target_gen = gvir_designer_domain_next_disk_target(design, bus); + if (!target_gen) { + g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0, + "unable to generate target name for bus '%s'", bus_str); + goto error; + } + } + gvir_config_domain_disk_set_target_dev(disk, target); + + g_list_free(bus_str_list); + g_free(target_gen); + + gvir_config_domain_add_device(priv->config, GVIR_CONFIG_DOMAIN_DEVICE(disk)); + + return disk; + +error: + g_free(target_gen); + g_list_free(bus_str_list); + if (disk) + g_object_unref(disk); + return NULL; +} +/** + * gvir_designer_domain_add_disk_file: + * @design: (transfer none): the domain designer instance + * @filepath: (transfer none): the path to a file + * @format: (transfer none): disk format + * + * Add a new disk to the domain. + * + * Returns: (transfer none): the pointer to new disk. + * If something fails NULL is returned and @error is set. + */ +GVirConfigDomainDisk *gvir_designer_domain_add_disk_file(GVirDesignerDomain *design, + const char *filepath, + const char *format, + GError **error) +{ + g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL); + + GVirConfigDomainDisk *ret = NULL; + + ret = gvir_designer_domain_add_disk_full(design, + GVIR_CONFIG_DOMAIN_DISK_FILE, + filepath, + format, + NULL, + error); + return ret; +} + +/** + * gvir_designer_domain_add_disk_device: + * @design: (transfer none): the domain designer instance + * @devpath: (transfer none): path to the device + * + * Add given device as a new disk to the domain designer instance. + * + * Returns: (transfer none): the pointer to the new disk. + * If something fails NULL is returned and @error is set. + */ +GVirConfigDomainDisk *gvir_designer_domain_add_disk_device(GVirDesignerDomain *design, + const char *devpath, + GError **error) +{ + g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL); + + GVirConfigDomainDisk *ret = NULL; + + ret = gvir_designer_domain_add_disk_full(design, + GVIR_CONFIG_DOMAIN_DISK_BLOCK, + devpath, + "raw", + NULL, + error); + return ret; +} diff --git a/libvirt-designer/libvirt-designer-domain.h b/libvirt-designer/libvirt-designer-domain.h index 23b5750..06a5749 100644 --- a/libvirt-designer/libvirt-designer-domain.h +++ b/libvirt-designer/libvirt-designer-domain.h @@ -93,6 +93,13 @@ gboolean gvir_designer_domain_setup_container_full(GVirDesignerDomain *design, const char *arch, GError **error); +GVirConfigDomainDisk *gvir_designer_domain_add_disk_file(GVirDesignerDomain *design, + const char *filepath, + const char *format, + GError **error); +GVirConfigDomainDisk *gvir_designer_domain_add_disk_device(GVirDesignerDomain *design, + const char *devpath, + GError **error); G_END_DECLS diff --git a/libvirt-designer/libvirt-designer.sym b/libvirt-designer/libvirt-designer.sym index 2a56a92..e67323a 100644 --- a/libvirt-designer/libvirt-designer.sym +++ b/libvirt-designer/libvirt-designer.sym @@ -10,6 +10,9 @@ LIBVIRT_DESIGNER_0.0.1 { gvir_designer_domain_get_platform; gvir_designer_domain_get_capabilities; + gvir_designer_domain_add_disk_file; + gvir_designer_domain_add_disk_device; + gvir_designer_domain_setup_machine; gvir_designer_domain_setup_machine_full; gvir_designer_domain_setup_container; -- 1.7.8.6
On 09/05/2012 11:27 AM, Michal Privoznik wrote:
Let users add either files or devices as disks to domains. --- libvirt-designer/libvirt-designer-domain.c | 244 ++++++++++++++++++++++++++++ libvirt-designer/libvirt-designer-domain.h | 7 + libvirt-designer/libvirt-designer.sym | 3 + 3 files changed, 254 insertions(+), 0 deletions(-)
diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index a8cabde..20611f2 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c [...] @@ -663,3 +670,240 @@ cleanup: g_object_unref(guest); return ret; } + +static GList * +gvir_designer_domain_get_supported_disk_bus_types(GVirDesignerDomain *design) +{ + GVirDesignerDomainPrivate *priv = design->priv; + OsinfoDeviceList *dev_list; + GHashTable *bus_hash = g_hash_table_new(g_str_hash, g_str_equal); + GList *ret = NULL; + int i; + + dev_list = osinfo_os_get_devices_by_property(priv->os, "class", "block", TRUE); + + for (i = 0; i < osinfo_list_get_length(OSINFO_LIST(dev_list)); i++) { + OsinfoDevice *dev = OSINFO_DEVICE(osinfo_list_get_nth(OSINFO_LIST(dev_list), i)); + const gchar *bus = osinfo_device_get_bus_type(dev); + + if (bus) + g_hash_table_add(bus_hash, g_strdup(bus)); + } + + ret = g_hash_table_get_keys(bus_hash); + ret = g_list_copy(ret);
It's probably OK here...
+ + g_hash_table_destroy(bus_hash);
...and here, but...
+ return ret; +} + [...] +static const gchar * +gvir_designer_domain_get_preferred_disk_bus_type(GVirDesignerDomain *design, + GError **error) +{ + OsinfoDevice *dev; + OsinfoDeviceLink *dev_link = gvir_designer_domain_get_preferred_device(design, + "block", + error); + const gchar *ret = NULL; + + if (!dev_link) + return NULL; + + dev = osinfo_devicelink_get_target(dev_link); + ret = osinfo_device_get_bus_type(dev);
...for example here, shouldn't this be checked for the value passed to the function? I know osinfo uses asserts for some of these, but that's hard to say which of these should be checked and what's the right procedure as lots of programs throw out these. I personally don't like these "<gibberish> CRITICAL <gibberish>" lines at all. But I'll get to that in another patch anyway ;) Other than that this patch seems decent, even though I didn't do such a thorough investigation as I do with other patches as this is pretty new project. Martin
--- .gitignore | 1 + Makefile.am | 2 +- configure.ac | 6 +- examples/Makefile.am | 23 ++++ examples/virtxml.c | 334 ++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 364 insertions(+), 2 deletions(-) create mode 100644 examples/Makefile.am create mode 100644 examples/virtxml.c diff --git a/.gitignore b/.gitignore index b7ba45a..d570af8 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ Makefile.in /config.log /config.status /configure +/examples/virtxml /libtool /libvirt-designer-1.0.pc /libvirt-designer.spec diff --git a/Makefile.am b/Makefile.am index b0f68c0..ab06626 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,5 +1,5 @@ -SUBDIRS = libvirt-designer +SUBDIRS = libvirt-designer examples ACLOCAL_AMFLAGS = -I m4 diff --git a/configure.ac b/configure.ac index 795990f..ebe7b35 100644 --- a/configure.ac +++ b/configure.ac @@ -13,6 +13,7 @@ AM_SILENT_RULES([yes]) LIBOSINFO_REQUIRED=0.0.5 LIBVIRT_GCONFIG_REQUIRED=0.0.9 GOBJECT_INTROSPECTION_REQUIRED=0.10.8 +LIBVIRT_REQUIRED=0.9.0 LIBVIRT_DESIGNER_MAJOR_VERSION=`echo $VERSION | awk -F. '{print $1}'` LIBVIRT_DESIGNER_MINOR_VERSION=`echo $VERSION | awk -F. '{print $2}'` @@ -40,6 +41,7 @@ LIBVIRT_DESIGNER_COMPILE_WARNINGS PKG_CHECK_MODULES(LIBOSINFO, libosinfo-1.0 >= $LIBOSINFO_REQUIRED) PKG_CHECK_MODULES(LIBVIRT_GCONFIG, libvirt-gconfig-1.0 >= $LIBVIRT_GCONFIG_REQUIRED) +PKG_CHECK_MODULES(LIBVIRT, libvirt >= $LIBVIRT_REQUIRED) LIBVIRT_DESIGNER_GETTEXT LIBVIRT_DESIGNER_GTK_MISC @@ -51,7 +53,8 @@ LIBVIRT_DESIGNER_INTROSPECTION AC_OUTPUT(Makefile libvirt-designer/Makefile libvirt-designer.spec - libvirt-designer-1.0.pc) + libvirt-designer-1.0.pc + examples/Makefile) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Configuration summary]) @@ -62,4 +65,5 @@ AC_MSG_NOTICE([ Libraries:]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ LIBOSINFO: $LIBOSINFO_CFLAGS $LIBOSINFO_LIBS]) AC_MSG_NOTICE([ LIBVIRT_GCONFIG: $LIBVIRT_GCONFIG_CFLAGS $LIBVIRT_GCONFIG_LIBS]) +AC_MSG_NOTICE([ LIBVIRT: $LIBVIRT_CFLAGS $LIBVIRT_LIBS]) AC_MSG_NOTICE([]) diff --git a/examples/Makefile.am b/examples/Makefile.am new file mode 100644 index 0000000..cc2b997 --- /dev/null +++ b/examples/Makefile.am @@ -0,0 +1,23 @@ +INCLUDES = \ + -I$(top_builddir)/libvirt-designer \ + -I$(top_srcdir) + +virtxml_LDADD = \ + $(top_builddir)/libvirt-designer/libvirt-designer-1.0.la + +virtxml_CFLAGS = \ + -I$(top_srcdir) \ + $(COVERAGE_CFLAGS) \ + -I$(top_srcdir) \ + $(LIBOSINFO_CFLAGS) \ + $(LIBVIRT_GCONFIG_CFLAGS) \ + $(WARN_CFLAGS2) \ + $(LIBVIRT_CFLAGS) \ + $(NULL) + +virtxml_LDFLAGS = \ + $(LIBOSINFO_LIBS) \ + $(LIBVIRT_GCONFIG_LIBS) \ + $(LIBVIRT_LIBS) + +bin_PROGRAMS = virtxml diff --git a/examples/virtxml.c b/examples/virtxml.c new file mode 100644 index 0000000..36bc3bb --- /dev/null +++ b/examples/virtxml.c @@ -0,0 +1,334 @@ +/* + * virtxml.c: produce an domain XML + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> +#include <libvirt-designer/libvirt-designer.h> +#include <libvirt/libvirt.h> +#include <libvirt/virterror.h> + +#include <stdio.h> +#include <getopt.h> +#include <stdlib.h> +#include <stdarg.h> +#include <string.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <unistd.h> + +#define print_error(...) \ + print_error_impl(__FUNCTION__, __LINE__, __VA_ARGS__) + +static void +print_error_impl(const char *funcname, + size_t linenr, + const char *fmt, ...) +{ + va_list args; + + fprintf(stderr, "Error in %s:%zu ", funcname, linenr); + va_start(args, fmt); + vfprintf(stderr, fmt, args); + va_end(args); + fprintf(stderr,"\n"); +} + +static void +print_usage(const char *progname) +{ + printf("\nUsage: %s options ...\n" + " options:\n" + " -h | --help print this help\n" + " -c | --connect=URI libvirt connection URI used \n" + " for querying capabilities\n" + " --list-os list IDs of known OSes\n" + " --list-platform list IDs of known hypervisors\n" + " -o | --os=OS set domain OS\n" + " -p | --platform=PLATFORM set hypervisor under which \n" + " domain will be running\n" + " -a | --architecture=ARCH set domain architecture\n" + " -d | --disk=PATH[,FORMAT] add disk to domain with PATH being \n" + " source and FORMAT is its format\n", + progname); +} + +static OsinfoDb * +get_default_osinfo_db(void) +{ + GError *err = NULL; + OsinfoLoader *loader = NULL; + OsinfoDb *ret = NULL; + + loader = osinfo_loader_new(); + osinfo_loader_process_default_path(loader, &err); + if (err) { + print_error("Unable to load default libosinfo DB: %s", err->message); + goto cleanup; + } + + ret = osinfo_loader_get_db(loader); + g_object_ref(ret); + +cleanup: + g_object_unref(loader); + return ret; +} + +static int +print_oses(void) +{ + OsinfoDb *db = get_default_osinfo_db(); + OsinfoOsList *list; + int i; + + if (!db) + return EXIT_FAILURE; + + list = osinfo_db_get_os_list(db); + + printf(" Operating System ID\n" + "-----------------------\n"); + for (i = 0 ; i < osinfo_list_get_length(OSINFO_LIST(list)) ; i++) { + OsinfoOs *ent = OSINFO_OS(osinfo_list_get_nth(OSINFO_LIST(list), i)); + const char *id = osinfo_entity_get_param_value(OSINFO_ENTITY(ent), + "short-id"); + if (!id) + id = osinfo_entity_get_param_value(OSINFO_ENTITY(ent), + OSINFO_ENTITY_PROP_ID); + printf("%s\n", id); + } + + g_object_unref(list); + g_object_unref(db); + + return EXIT_SUCCESS; +} + +static int +print_platforms(void) +{ + OsinfoDb *db = get_default_osinfo_db(); + OsinfoPlatformList *list; + int i; + + if (!db) + return EXIT_FAILURE; + + list = osinfo_db_get_platform_list(db); + + printf(" Platform ID\n" + "---------------\n"); + for (i = 0; i < osinfo_list_get_length(OSINFO_LIST(list)); i++) { + OsinfoPlatform *ent = OSINFO_PLATFORM(osinfo_list_get_nth(OSINFO_LIST(list), i)); + const char *id = osinfo_entity_get_param_value(OSINFO_ENTITY(ent), + "short-id"); + if (!id) + id = osinfo_entity_get_param_value(OSINFO_ENTITY(ent), + OSINFO_ENTITY_PROP_ID); + + printf("%s\n", id); + } + + g_object_unref(list); + g_object_unref(db); + + return EXIT_SUCCESS; +} + +static void +add_disk(gpointer data, + gpointer user_data) +{ + GVirDesignerDomain *domain = (GVirDesignerDomain *) user_data; + char *path = (char *) data; + char *format = NULL; + struct stat buf; + GError *error = NULL; + + format = strchr(path, ','); + if (format) { + *format = '\0'; + format++; + } + + if (!path || !strlen(path)) { + print_error("No path provided"); + return; + } + + if (!stat(path, &buf) && + !S_ISREG(buf.st_mode)) { + gvir_designer_domain_add_disk_device(domain, path, &error); + } else { + gvir_designer_domain_add_disk_file(domain, path, format, &error); + } + + if (error) + print_error("%s", error->message); + +} + +#define CHECK_ERROR \ + if (error) { \ + print_error("%s", error->message); \ + goto cleanup; \ + } + +int +main(int argc, char *argv[]) +{ + int ret = EXIT_FAILURE; + GError *error = NULL; + OsinfoOs *os = NULL; + OsinfoPlatform *platform = NULL; + GVirConfigCapabilities *caps = NULL; + GVirConfigDomain *config = NULL; + GVirDesignerDomain *domain = NULL; + virConnectPtr conn = NULL; + char *caps_str = NULL; + gchar *xml = NULL; + char *os_str = NULL; + char *platform_str = NULL; + char *arch_str = NULL; + char *connect_uri = NULL; + GList *disk_str_list = NULL; + int arg; + + struct option opt[] = { + {"help", no_argument, NULL, 'h'}, + {"connect", required_argument, NULL, 'c'}, + {"list-os", no_argument, NULL, 'O'}, + {"list-platform", no_argument, NULL, 'P'}, + {"os", required_argument, NULL, 'o'}, + {"platform", required_argument, NULL, 'p'}, + {"architecture", required_argument, NULL, 'a'}, + {"disk", required_argument, NULL, 'd'}, + {NULL, 0, NULL, 0} + }; + + if (!gvir_designer_init_check(&argc, &argv, NULL)) + return EXIT_FAILURE; + + /* Standard (non-command) options. The leading + ensures that no + * argument reordering takes place, so that command options are + * not confused with top-level virsh options. */ + while ((arg = getopt_long(argc, argv, "+hc:o:p:a:d:", opt, NULL)) != -1) { + char *progname; + switch (arg) { + case 'h': + if (!(progname = strrchr(argv[0], '/'))) + progname = argv[0]; + else + progname++; + + print_usage(progname); + exit(EXIT_SUCCESS); + break; + case 'c': + connect_uri = optarg; + break; + case 'O': + ret = print_oses(); + exit(ret); + break; + case 'P': + ret = print_platforms(); + exit(ret); + break; + case 'o': + if (os_str) { + print_error("Os already set to '%s'", os_str); + exit(EXIT_FAILURE); + } + os_str = optarg; + break; + case 'p': + if (platform_str) { + print_error("Platform already set to '%s'", platform_str); + exit(EXIT_FAILURE); + } + platform_str = optarg; + break; + case 'a': + if (arch_str) { + print_error("Architecture already set to '%s'", arch_str); + exit(EXIT_FAILURE); + } + arch_str = optarg; + break; + case 'd': + disk_str_list = g_list_append(disk_str_list, optarg); + break; + default: + print_error("Something has gone tragically wrong"); + case '?': + /* getopt_long already reported error */ + exit(EXIT_FAILURE); + } + } + + if (!os_str) { + print_error("Operating system was not specified"); + exit(EXIT_FAILURE); + } + if (!platform_str) { + print_error("Platform was not specified"); + exit(EXIT_FAILURE); + } + + conn = virConnectOpenAuth(connect_uri, virConnectAuthPtrDefault, VIR_CONNECT_RO); + if (!conn) { + print_error("Unable to connect to libvirt"); + return EXIT_FAILURE; + } + + if ((caps_str = virConnectGetCapabilities(conn)) == NULL) { + print_error("failed to get capabilities"); + goto cleanup; + } + + os = osinfo_os_new(os_str); + platform = osinfo_platform_new(platform_str); + caps = gvir_config_capabilities_new_from_xml(caps_str, NULL); + + domain = gvir_designer_domain_new(os, platform, caps); + + gvir_designer_domain_setup_machine(domain, &error); + CHECK_ERROR; + + if (arch_str) { + gvir_designer_domain_setup_container_full(domain, arch_str, &error); + CHECK_ERROR; + } + + g_list_foreach(disk_str_list, add_disk, domain); + + config = gvir_designer_domain_get_config(domain); + xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(config)); + + g_printf("%s\n", xml); + + ret = EXIT_SUCCESS; + +cleanup: + virConnectClose(conn); + return ret; +} -- 1.7.8.6
On 05.09.2012 11:27, Michal Privoznik wrote:
--- .gitignore | 1 + Makefile.am | 2 +- configure.ac | 6 +- examples/Makefile.am | 23 ++++ examples/virtxml.c | 334 ++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 364 insertions(+), 2 deletions(-) create mode 100644 examples/Makefile.am create mode 100644 examples/virtxml.c
Oh, I forgot to update configure.ac to check for newly introduced functions: diff --git a/configure.ac b/configure.ac index ebe7b35..bdee845 100644 --- a/configure.ac +++ b/configure.ac @@ -31,6 +31,12 @@ AC_SUBST([LIBVIRT_DESIGNER_VERSION_NUMBER]) AC_PROG_CC AM_PROG_CC_C_O +AC_CHECK_FUNCS([strchr]) +AC_CHECK_FUNCS([strrchr]) +AC_CHECK_FUNCS([uname]) +AC_PROG_CXX +AC_PROG_RANLIB +AC_TYPE_SIZE_T AC_LIBTOOL_WIN32_DLL AC_PROG_LIBTOOL Will squash in just before pushing. Michal
On 09/05/2012 11:27 AM, Michal Privoznik wrote:
--- .gitignore | 1 + Makefile.am | 2 +- configure.ac | 6 +- examples/Makefile.am | 23 ++++ examples/virtxml.c | 334 ++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 364 insertions(+), 2 deletions(-) create mode 100644 examples/Makefile.am create mode 100644 examples/virtxml.c
diff --git a/.gitignore b/.gitignore index b7ba45a..d570af8 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ Makefile.in /config.log /config.status /configure +/examples/virtxml /libtool /libvirt-designer-1.0.pc /libvirt-designer.spec diff --git a/Makefile.am b/Makefile.am index b0f68c0..ab06626 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,5 +1,5 @@
-SUBDIRS = libvirt-designer +SUBDIRS = libvirt-designer examples
ACLOCAL_AMFLAGS = -I m4
diff --git a/configure.ac b/configure.ac index 795990f..ebe7b35 100644 --- a/configure.ac +++ b/configure.ac @@ -13,6 +13,7 @@ AM_SILENT_RULES([yes]) LIBOSINFO_REQUIRED=0.0.5 LIBVIRT_GCONFIG_REQUIRED=0.0.9 GOBJECT_INTROSPECTION_REQUIRED=0.10.8 +LIBVIRT_REQUIRED=0.9.0
LIBVIRT_DESIGNER_MAJOR_VERSION=`echo $VERSION | awk -F. '{print $1}'` LIBVIRT_DESIGNER_MINOR_VERSION=`echo $VERSION | awk -F. '{print $2}'` @@ -40,6 +41,7 @@ LIBVIRT_DESIGNER_COMPILE_WARNINGS
PKG_CHECK_MODULES(LIBOSINFO, libosinfo-1.0 >= $LIBOSINFO_REQUIRED) PKG_CHECK_MODULES(LIBVIRT_GCONFIG, libvirt-gconfig-1.0 >= $LIBVIRT_GCONFIG_REQUIRED) +PKG_CHECK_MODULES(LIBVIRT, libvirt >= $LIBVIRT_REQUIRED)
LIBVIRT_DESIGNER_GETTEXT LIBVIRT_DESIGNER_GTK_MISC @@ -51,7 +53,8 @@ LIBVIRT_DESIGNER_INTROSPECTION AC_OUTPUT(Makefile libvirt-designer/Makefile libvirt-designer.spec - libvirt-designer-1.0.pc) + libvirt-designer-1.0.pc + examples/Makefile)
AC_MSG_NOTICE([]) AC_MSG_NOTICE([Configuration summary]) @@ -62,4 +65,5 @@ AC_MSG_NOTICE([ Libraries:]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ LIBOSINFO: $LIBOSINFO_CFLAGS $LIBOSINFO_LIBS]) AC_MSG_NOTICE([ LIBVIRT_GCONFIG: $LIBVIRT_GCONFIG_CFLAGS $LIBVIRT_GCONFIG_LIBS]) +AC_MSG_NOTICE([ LIBVIRT: $LIBVIRT_CFLAGS $LIBVIRT_LIBS]) AC_MSG_NOTICE([]) diff --git a/examples/Makefile.am b/examples/Makefile.am new file mode 100644 index 0000000..cc2b997 --- /dev/null +++ b/examples/Makefile.am @@ -0,0 +1,23 @@ +INCLUDES = \ + -I$(top_builddir)/libvirt-designer \ + -I$(top_srcdir) + +virtxml_LDADD = \ + $(top_builddir)/libvirt-designer/libvirt-designer-1.0.la + +virtxml_CFLAGS = \ + -I$(top_srcdir) \ + $(COVERAGE_CFLAGS) \ + -I$(top_srcdir) \
You've got this line twice here, plus it's already mentioned in INCLUDES. [...]
+static OsinfoDb * +get_default_osinfo_db(void) +{ + GError *err = NULL; + OsinfoLoader *loader = NULL; + OsinfoDb *ret = NULL; + + loader = osinfo_loader_new(); + osinfo_loader_process_default_path(loader, &err); + if (err) { + print_error("Unable to load default libosinfo DB: %s", err->message); + goto cleanup; + } + + ret = osinfo_loader_get_db(loader); + g_object_ref(ret);
I wonder if this needs to be done. Looking at the libosinfo code it doesn't seem to do that anywhere, but since you unref the loader right after that (which could lead to the db being unreferenced), I think it is needed here.
+ +cleanup: + g_object_unref(loader); + return ret; +} + +static int +print_oses(void) +{ + OsinfoDb *db = get_default_osinfo_db(); + OsinfoOsList *list; + int i; + + if (!db) + return EXIT_FAILURE; + + list = osinfo_db_get_os_list(db); + + printf(" Operating System ID\n" + "-----------------------\n"); + for (i = 0 ; i < osinfo_list_get_length(OSINFO_LIST(list)) ; i++) { + OsinfoOs *ent = OSINFO_OS(osinfo_list_get_nth(OSINFO_LIST(list), i)); + const char *id = osinfo_entity_get_param_value(OSINFO_ENTITY(ent), + "short-id"); + if (!id) + id = osinfo_entity_get_param_value(OSINFO_ENTITY(ent), + OSINFO_ENTITY_PROP_ID); + printf("%s\n", id); + }
Just a thought: Can this be optimized a bit? There is a function that returns linked list of all the elements. Is it usable? [...]
+static void +add_disk(gpointer data, + gpointer user_data) +{ + GVirDesignerDomain *domain = (GVirDesignerDomain *) user_data; + char *path = (char *) data; + char *format = NULL; + struct stat buf; + GError *error = NULL; + + format = strchr(path, ','); + if (format) { + *format = '\0'; + format++; + } + + if (!path || !strlen(path)) { + print_error("No path provided"); + return; + } + + if (!stat(path, &buf) && + !S_ISREG(buf.st_mode)) { + gvir_designer_domain_add_disk_device(domain, path, &error); + } else { + gvir_designer_domain_add_disk_file(domain, path, format, &error); + }
This means that if the file exists and is not a regular file, then it is added as a device, otherwise (e.g. the file doesn't exist), it is added as a file, right? I have no problem with that, I just want to confirm I understood this well. There might be some problems with that, but as it is just an example it doesn't matter that much.
+ + if (error) + print_error("%s", error->message); + +} + +#define CHECK_ERROR \ + if (error) { \ + print_error("%s", error->message); \ + goto cleanup; \ + } + +int +main(int argc, char *argv[]) +{ + int ret = EXIT_FAILURE; + GError *error = NULL; + OsinfoOs *os = NULL; + OsinfoPlatform *platform = NULL; + GVirConfigCapabilities *caps = NULL; + GVirConfigDomain *config = NULL; + GVirDesignerDomain *domain = NULL; + virConnectPtr conn = NULL; + char *caps_str = NULL; + gchar *xml = NULL; + char *os_str = NULL; + char *platform_str = NULL; + char *arch_str = NULL; + char *connect_uri = NULL; + GList *disk_str_list = NULL; + int arg; + + struct option opt[] = { + {"help", no_argument, NULL, 'h'}, + {"connect", required_argument, NULL, 'c'}, + {"list-os", no_argument, NULL, 'O'}, + {"list-platform", no_argument, NULL, 'P'}, + {"os", required_argument, NULL, 'o'}, + {"platform", required_argument, NULL, 'p'}, + {"architecture", required_argument, NULL, 'a'}, + {"disk", required_argument, NULL, 'd'}, + {NULL, 0, NULL, 0} + }; + + if (!gvir_designer_init_check(&argc, &argv, NULL)) + return EXIT_FAILURE; + + /* Standard (non-command) options. The leading + ensures that no + * argument reordering takes place, so that command options are + * not confused with top-level virsh options. */ + while ((arg = getopt_long(argc, argv, "+hc:o:p:a:d:", opt, NULL)) != -1) { + char *progname; + switch (arg) { + case 'h': + if (!(progname = strrchr(argv[0], '/'))) + progname = argv[0]; + else + progname++; + + print_usage(progname); + exit(EXIT_SUCCESS); + break; + case 'c': + connect_uri = optarg; + break; + case 'O': + ret = print_oses(); + exit(ret); + break; + case 'P': + ret = print_platforms(); + exit(ret); + break; + case 'o': + if (os_str) { + print_error("Os already set to '%s'", os_str); + exit(EXIT_FAILURE); + } + os_str = optarg; + break; + case 'p': + if (platform_str) { + print_error("Platform already set to '%s'", platform_str); + exit(EXIT_FAILURE); + } + platform_str = optarg; + break; + case 'a': + if (arch_str) { + print_error("Architecture already set to '%s'", arch_str); + exit(EXIT_FAILURE); + } + arch_str = optarg; + break; + case 'd': + disk_str_list = g_list_append(disk_str_list, optarg); + break; + default: + print_error("Something has gone tragically wrong"); + case '?': + /* getopt_long already reported error */ + exit(EXIT_FAILURE); + } + } + + if (!os_str) { + print_error("Operating system was not specified"); + exit(EXIT_FAILURE); + } + if (!platform_str) { + print_error("Platform was not specified"); + exit(EXIT_FAILURE); + } + + conn = virConnectOpenAuth(connect_uri, virConnectAuthPtrDefault, VIR_CONNECT_RO); + if (!conn) { + print_error("Unable to connect to libvirt"); + return EXIT_FAILURE; + } + + if ((caps_str = virConnectGetCapabilities(conn)) == NULL) { + print_error("failed to get capabilities"); + goto cleanup; + } + + os = osinfo_os_new(os_str);
You don't check if the OS is found...
+ platform = osinfo_platform_new(platform_str); + caps = gvir_config_capabilities_new_from_xml(caps_str, NULL); + + domain = gvir_designer_domain_new(os, platform, caps);
...which leads to another "CRITICAL" reported here. I've found this out by trying to use this and this happened even though the listed OS IDs (short IDs if possible if I understood the code above). But long IDs work.
+ + gvir_designer_domain_setup_machine(domain, &error); + CHECK_ERROR; + + if (arch_str) { + gvir_designer_domain_setup_container_full(domain, arch_str, &error); + CHECK_ERROR; + } + + g_list_foreach(disk_str_list, add_disk, domain); + + config = gvir_designer_domain_get_config(domain); + xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(config)); + + g_printf("%s\n", xml); + + ret = EXIT_SUCCESS; + +cleanup: + virConnectClose(conn); + return ret; +}
Plus you mix return EXIT_FAILURE and exit(EXIT_FAILURE) in the main a lot. Martin
Hey, I haven't looked at the patch in depth, just a few comments: On Wed, Sep 05, 2012 at 11:27:46AM +0200, Michal Privoznik wrote:
+ + /* Standard (non-command) options. The leading + ensures that no + * argument reordering takes place, so that command options are + * not confused with top-level virsh options. */ + while ((arg = getopt_long(argc, argv, "+hc:o:p:a:d:", opt, NULL)) != -1) {
glib has its own commandline parser http://developer.gnome.org/glib/stable/glib-Commandline-option-parser.html I'm not saying you have to use it, I'm merely mentioning it in case you prefer something different from getopt.
+ char *progname; + switch (arg) { + case 'h': + if (!(progname = strrchr(argv[0], '/'))) + progname = argv[0]; + else + progname++;
You can also use g_path_get_basename
+ + conn = virConnectOpenAuth(connect_uri, virConnectAuthPtrDefault, VIR_CONNECT_RO); + if (!conn) { + print_error("Unable to connect to libvirt"); + return EXIT_FAILURE; + } + + if ((caps_str = virConnectGetCapabilities(conn)) == NULL) { + print_error("failed to get capabilities"); + goto cleanup; + }
libvirt-glib has gvir_connection_get_capabilities, I'm not sure about virConnectOpenAuth. With that said, I guess not using libvirt-gobject is done on purpose as libvirt-designer is not depending on it, right? Christophe
Let users add NICs to domains. --- examples/virtxml.c | 96 ++++++++++++++++++++++++---- libvirt-designer/libvirt-designer-domain.c | 53 +++++++++++++++ libvirt-designer/libvirt-designer-domain.h | 3 + libvirt-designer/libvirt-designer.sym | 1 + 4 files changed, 141 insertions(+), 12 deletions(-) diff --git a/examples/virtxml.c b/examples/virtxml.c index 36bc3bb..87929b2 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -56,17 +56,19 @@ print_usage(const char *progname) { printf("\nUsage: %s options ...\n" " options:\n" - " -h | --help print this help\n" - " -c | --connect=URI libvirt connection URI used \n" - " for querying capabilities\n" - " --list-os list IDs of known OSes\n" - " --list-platform list IDs of known hypervisors\n" - " -o | --os=OS set domain OS\n" - " -p | --platform=PLATFORM set hypervisor under which \n" - " domain will be running\n" - " -a | --architecture=ARCH set domain architecture\n" - " -d | --disk=PATH[,FORMAT] add disk to domain with PATH being \n" - " source and FORMAT is its format\n", + " -h | --help print this help\n" + " -c | --connect=URI libvirt connection URI used \n" + " for querying capabilities\n" + " --list-os list IDs of known OSes\n" + " --list-platform list IDs of known hypervisors\n" + " -o | --os=OS set domain OS\n" + " -p | --platform=PLATFORM set hypervisor under which \n" + " domain will be running\n" + " -a | --architecture=ARCH set domain architecture\n" + " -d | --disk=PATH[,FORMAT] add disk to domain with PATH being \n" + " source and FORMAT is its format\n" + " -i | --interface=NETWORK[,ARG=VAL] add interface with NETWORK source.\n" + " Possible ARGs: mac,link={up,down}\n", progname); } @@ -186,6 +188,69 @@ add_disk(gpointer data, } +static void +add_iface(gpointer data, + gpointer user_data) +{ + GVirDesignerDomain *domain = (GVirDesignerDomain *) user_data; + char *network = (char *) data; + char *param = NULL; + GVirConfigDomainInterface *iface = NULL; + GError *error = NULL; + + param = strchr(network, ','); + if (param) { + *param = '\0'; + param++; + } + + iface = gvir_designer_domain_add_interface_network(domain, network, &error); + if (error) { + print_error("%s", error->message); + exit(EXIT_FAILURE); + } + + while (param && *param) { + char *key = param; + char *val; + GVirConfigDomainInterfaceLinkState link; + + /* move to next token */ + param = strchr(param, ','); + if (param) { + *param = '\0'; + param++; + } + + /* parse token */ + val = strchr(key, '='); + if (!val) { + print_error("Invalid format: %s", key); + exit(EXIT_FAILURE); + } + + *val = '\0'; + val++; + + if (!strcmp(key, "mac")) { + gvir_config_domain_interface_set_mac(iface, val); + } else if (!strcmp(key, "link")) { + if (!strcmp(val, "up")) { + link = GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_UP; + } else if (!strcmp(val, "down")) { + link = GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DOWN; + } else { + print_error("Unknown value: %s", val); + exit(EXIT_FAILURE); + } + gvir_config_domain_interface_set_link_state(iface, link); + } else { + print_error("Unknown key: %s", key); + exit(EXIT_FAILURE); + } + } +} + #define CHECK_ERROR \ if (error) { \ print_error("%s", error->message); \ @@ -210,6 +275,7 @@ main(int argc, char *argv[]) char *arch_str = NULL; char *connect_uri = NULL; GList *disk_str_list = NULL; + GList *iface_str_list = NULL; int arg; struct option opt[] = { @@ -221,6 +287,7 @@ main(int argc, char *argv[]) {"platform", required_argument, NULL, 'p'}, {"architecture", required_argument, NULL, 'a'}, {"disk", required_argument, NULL, 'd'}, + {"interface", required_argument, NULL, 'i'}, {NULL, 0, NULL, 0} }; @@ -230,7 +297,7 @@ main(int argc, char *argv[]) /* Standard (non-command) options. The leading + ensures that no * argument reordering takes place, so that command options are * not confused with top-level virsh options. */ - while ((arg = getopt_long(argc, argv, "+hc:o:p:a:d:", opt, NULL)) != -1) { + while ((arg = getopt_long(argc, argv, "+hc:o:p:a:d:i:", opt, NULL)) != -1) { char *progname; switch (arg) { case 'h': @@ -277,6 +344,9 @@ main(int argc, char *argv[]) case 'd': disk_str_list = g_list_append(disk_str_list, optarg); break; + case 'i': + iface_str_list = g_list_append(iface_str_list, optarg); + break; default: print_error("Something has gone tragically wrong"); case '?': @@ -321,6 +391,8 @@ main(int argc, char *argv[]) g_list_foreach(disk_str_list, add_disk, domain); + g_list_foreach(iface_str_list, add_iface, domain); + config = gvir_designer_domain_get_config(domain); xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(config)); diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 20611f2..5a10836 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -907,3 +907,56 @@ GVirConfigDomainDisk *gvir_designer_domain_add_disk_device(GVirDesignerDomain *d error); return ret; } + +static const gchar * +gvir_designer_domain_get_preferred_nic_model(GVirDesignerDomain *design, + GError **error) +{ + const gchar *ret = NULL; + OsinfoDeviceLink *link = NULL; + + link = gvir_designer_domain_get_preferred_device(design, "network", error); + if (!link) + goto cleanup; + + ret = osinfo_devicelink_get_driver(link); + +cleanup: + if (link) + g_object_unref(link); + return ret; +} + +/** + * gvir_designer_domain_add_interface_network: + * @design: (transfer none): the domain designer instance + * @network: (transfer none): network name + * + * Add new network interface card into @design. The interface is + * of 'network' type with @network used as the source network. + * + * Returns: (transfer none): the pointer to the new interface. + */ +GVirConfigDomainInterface * +gvir_designer_domain_add_interface_network(GVirDesignerDomain *design, + const char *network, + GError **error) +{ + g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL); + + GVirConfigDomainInterface *ret; + const gchar *model = NULL; + + model = gvir_designer_domain_get_preferred_nic_model(design, error); + + 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); + if (model) + gvir_config_domain_interface_set_model(ret, model); + + gvir_config_domain_add_device(design->priv->config, GVIR_CONFIG_DOMAIN_DEVICE(ret)); + + return ret; +} diff --git a/libvirt-designer/libvirt-designer-domain.h b/libvirt-designer/libvirt-designer-domain.h index 06a5749..5097393 100644 --- a/libvirt-designer/libvirt-designer-domain.h +++ b/libvirt-designer/libvirt-designer-domain.h @@ -101,6 +101,9 @@ GVirConfigDomainDisk *gvir_designer_domain_add_disk_device(GVirDesignerDomain *d const char *devpath, GError **error); +GVirConfigDomainInterface *gvir_designer_domain_add_interface_network(GVirDesignerDomain *design, + const char *network, + GError **error); G_END_DECLS #endif /* __LIBVIRT_DESIGNER_DOMAIN_H__ */ diff --git a/libvirt-designer/libvirt-designer.sym b/libvirt-designer/libvirt-designer.sym index e67323a..77f76b4 100644 --- a/libvirt-designer/libvirt-designer.sym +++ b/libvirt-designer/libvirt-designer.sym @@ -12,6 +12,7 @@ LIBVIRT_DESIGNER_0.0.1 { gvir_designer_domain_add_disk_file; gvir_designer_domain_add_disk_device; + gvir_designer_domain_add_interface_network; gvir_designer_domain_setup_machine; gvir_designer_domain_setup_machine_full; -- 1.7.8.6
On 09/05/2012 11:27 AM, Michal Privoznik wrote:
Let users add NICs to domains. --- examples/virtxml.c | 96 ++++++++++++++++++++++++---- libvirt-designer/libvirt-designer-domain.c | 53 +++++++++++++++ libvirt-designer/libvirt-designer-domain.h | 3 + libvirt-designer/libvirt-designer.sym | 1 + 4 files changed, 141 insertions(+), 12 deletions(-)
[...]
+ +static const gchar * +gvir_designer_domain_get_preferred_nic_model(GVirDesignerDomain *design, + GError **error) +{ + const gchar *ret = NULL; + OsinfoDeviceLink *link = NULL;
You are using "link" here which shadows some global declaration. I'm saying "some" because the compiler told me so, but after many files I wasn't so eager to find it, so I don't know which one. Anyway changing it to dev_link (as you use everywhere else) worked.
+ + link = gvir_designer_domain_get_preferred_device(design, "network", error); + if (!link) + goto cleanup; + + ret = osinfo_devicelink_get_driver(link); + +cleanup: + if (link) + g_object_unref(link); + return ret; +} + +/** + * gvir_designer_domain_add_interface_network: + * @design: (transfer none): the domain designer instance + * @network: (transfer none): network name + * + * Add new network interface card into @design. The interface is + * of 'network' type with @network used as the source network. + * + * Returns: (transfer none): the pointer to the new interface. + */ +GVirConfigDomainInterface * +gvir_designer_domain_add_interface_network(GVirDesignerDomain *design, + const char *network, + GError **error) +{
Are you planning on using gvir...add_interface_full with add_interface_{network,bridge,etc.} as "wrappers"? I liked that with the disk in the first patch.
+ g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL);
You check this value here but not on all the previous places (and patches).
+ + GVirConfigDomainInterface *ret; + const gchar *model = NULL; + + model = gvir_designer_domain_get_preferred_nic_model(design, error); + + ret = GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_network_new());
I can't find the function anywhere, even though it isn't defined automagically (IIUC), but it compiles (I wonder what the macro does in here).
+ + gvir_config_domain_interface_network_set_source(GVIR_CONFIG_DOMAIN_INTERFACE_NETWORK(ret), + network); + if (model) + gvir_config_domain_interface_set_model(ret, model); + + gvir_config_domain_add_device(design->priv->config, GVIR_CONFIG_DOMAIN_DEVICE(ret)); + + return ret; +} diff --git a/libvirt-designer/libvirt-designer-domain.h b/libvirt-designer/libvirt-designer-domain.h index 06a5749..5097393 100644 --- a/libvirt-designer/libvirt-designer-domain.h +++ b/libvirt-designer/libvirt-designer-domain.h @@ -101,6 +101,9 @@ GVirConfigDomainDisk *gvir_designer_domain_add_disk_device(GVirDesignerDomain *d const char *devpath, GError **error);
+GVirConfigDomainInterface *gvir_designer_domain_add_interface_network(GVirDesignerDomain *design, + const char *network, + GError **error); G_END_DECLS
#endif /* __LIBVIRT_DESIGNER_DOMAIN_H__ */ diff --git a/libvirt-designer/libvirt-designer.sym b/libvirt-designer/libvirt-designer.sym index e67323a..77f76b4 100644 --- a/libvirt-designer/libvirt-designer.sym +++ b/libvirt-designer/libvirt-designer.sym @@ -12,6 +12,7 @@ LIBVIRT_DESIGNER_0.0.1 {
gvir_designer_domain_add_disk_file; gvir_designer_domain_add_disk_device; + gvir_designer_domain_add_interface_network;
Indentation. Martin
Hey, On Fri, Sep 07, 2012 at 03:56:34PM +0200, Martin Kletzander wrote:
On 09/05/2012 11:27 AM, Michal Privoznik wrote:
+ +static const gchar * +gvir_designer_domain_get_preferred_nic_model(GVirDesignerDomain *design, + GError **error) +{ + const gchar *ret = NULL; + OsinfoDeviceLink *link = NULL;
You are using "link" here which shadows some global declaration. I'm saying "some" because the compiler told me so, but after many files I wasn't so eager to find it, so I don't know which one. Anyway changing it to dev_link (as you use everywhere else) worked.
Probably link(3) which creates hardlinks.
+ + GVirConfigDomainInterface *ret; + const gchar *model = NULL; + + model = gvir_designer_domain_get_preferred_nic_model(design, error); + + ret = GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_network_new());
I can't find the function anywhere, even though it isn't defined automagically (IIUC), but it compiles (I wonder what the macro does in here).
$prefix/include/libvirt-gconfig-1.0/libvirt-gconfig/libvirt-gconfig-interface.h has: #define GVIR_CONFIG_INTERFACE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_CONFIG_TYPE_INTERFACE, GVirConfigInterface)) which is C cast + runtime type check. This is used everywhere in GObject land. gvir_config_domain_interface_network_new is defined in libvirt-gconfig-domain-interface-network.h Christophe
On 09/10/2012 11:19 AM, Christophe Fergeau wrote:
Hey,
On Fri, Sep 07, 2012 at 03:56:34PM +0200, Martin Kletzander wrote:
On 09/05/2012 11:27 AM, Michal Privoznik wrote:
+ +static const gchar * +gvir_designer_domain_get_preferred_nic_model(GVirDesignerDomain *design, + GError **error) +{ + const gchar *ret = NULL; + OsinfoDeviceLink *link = NULL;
You are using "link" here which shadows some global declaration. I'm saying "some" because the compiler told me so, but after many files I wasn't so eager to find it, so I don't know which one. Anyway changing it to dev_link (as you use everywhere else) worked.
Probably link(3) which creates hardlinks.
Oh, right. I was looking at the code and it starts looking as something else than C sometimes. Thanks for "waking me up" :)
+ + GVirConfigDomainInterface *ret; + const gchar *model = NULL; + + model = gvir_designer_domain_get_preferred_nic_model(design, error); + + ret = GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_network_new());
I can't find the function anywhere, even though it isn't defined automagically (IIUC), but it compiles (I wonder what the macro does in here).
$prefix/include/libvirt-gconfig-1.0/libvirt-gconfig/libvirt-gconfig-interface.h has: #define GVIR_CONFIG_INTERFACE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_CONFIG_TYPE_INTERFACE, GVirConfigInterface)) which is C cast + runtime type check. This is used everywhere in GObject land. gvir_config_domain_interface_network_new is defined in libvirt-gconfig-domain-interface-network.h
Thanks, but I have to look inside this one, anyway. You confirmed me what I thought this is what the function is doing, but I saw no need for cast in here. Now I see it has to be changed from InterfaceNetwork to Interface. I guess this is some kind of polymorphism/inheritance design in glib. As I said before, I'm not very familiar with glib (and similar), I just wanted to check Michal's code and give it some attention :) Have a nice day, Martin
On Mon, Sep 10, 2012 at 11:38:30AM +0200, Martin Kletzander wrote:
On 09/10/2012 11:19 AM, Christophe Fergeau wrote: Thanks, but I have to look inside this one, anyway. You confirmed me what I thought this is what the function is doing, but I saw no need for cast in here. Now I see it has to be changed from InterfaceNetwork to Interface. I guess this is some kind of polymorphism/inheritance design in glib.
Yes, C being what it is, you cannot get automatic casts from a child class to one of its parents, so we have to do it explicitly when needed. Christophe
participants (3)
-
Christophe Fergeau -
Martin Kletzander -
Michal Privoznik