[libvirt] [libvirt-designer][PATCH 3/4] examples: Create an example of usage program
Martin Kletzander
mkletzan at redhat.com
Fri Sep 7 13:42:43 UTC 2012
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
More information about the libvir-list
mailing list