[libvirt] [libvirt-designer][PATCH v2 0/4] Yet another functional extension

This time aimed on virtxml tool to make it user friendlier a little bit. Michal Privoznik (4): virtxml: Init variables virtxml: Detect OS from given ISO virtxml: Detect platform from libvirt connection URI Implement resources setting configure.ac | 2 +- examples/virtxml.c | 260 +++++++++++++++++++++++++--- libvirt-designer/libvirt-designer-domain.c | 91 ++++++++++ libvirt-designer/libvirt-designer-domain.h | 8 + libvirt-designer/libvirt-designer.sym | 1 + 5 files changed, 335 insertions(+), 27 deletions(-) -- 1.7.8.6

to avoid their uninitialized usage. --- examples/virtxml.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/virtxml.c b/examples/virtxml.c index 9783ba6..b0c3a77 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -97,7 +97,7 @@ print_oses(const gchar *option_name, GError **error) { OsinfoDb *db = get_default_osinfo_db(); - OsinfoOsList *list; + OsinfoOsList *list = NULL; GList *oses = NULL; GList *os_iter; int ret = EXIT_FAILURE; @@ -140,7 +140,7 @@ print_platforms(const gchar *option_name, GError **error) { OsinfoDb *db = get_default_osinfo_db(); - OsinfoPlatformList *list; + OsinfoPlatformList *list = NULL; GList *platforms = NULL; GList *platform_iter; int ret = EXIT_FAILURE; -- 1.7.8.6

On Thu, Sep 13, 2012 at 04:04:28PM +0200, Michal Privoznik wrote:
to avoid their uninitialized usage.
ACK, I've sent a similar patch but forgot --subject-prefix for some reason... https://www.redhat.com/archives/libvir-list/2012-September/msg00804.html Let's use yours. Christophe
--- examples/virtxml.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/examples/virtxml.c b/examples/virtxml.c index 9783ba6..b0c3a77 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -97,7 +97,7 @@ print_oses(const gchar *option_name, GError **error) { OsinfoDb *db = get_default_osinfo_db(); - OsinfoOsList *list; + OsinfoOsList *list = NULL; GList *oses = NULL; GList *os_iter; int ret = EXIT_FAILURE; @@ -140,7 +140,7 @@ print_platforms(const gchar *option_name, GError **error) { OsinfoDb *db = get_default_osinfo_db(); - OsinfoPlatformList *list; + OsinfoPlatformList *list = NULL; GList *platforms = NULL; GList *platform_iter; int ret = EXIT_FAILURE; -- 1.7.8.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 14.09.2012 17:35, Christophe Fergeau wrote:
On Thu, Sep 13, 2012 at 04:04:28PM +0200, Michal Privoznik wrote:
to avoid their uninitialized usage.
ACK, I've sent a similar patch but forgot --subject-prefix for some reason... https://www.redhat.com/archives/libvir-list/2012-September/msg00804.html Let's use yours.
Christophe
Thanks, I've pushed whole series except 4/4 which I'll send in a while. Michal

In some cases telling OS version is redundant as ISO image with specified OS is passed some arguments later as disk. Don't require --os then. --- examples/virtxml.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 105 insertions(+), 5 deletions(-) diff --git a/examples/virtxml.c b/examples/virtxml.c index b0c3a77..4ec9cd8 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -293,6 +293,98 @@ add_iface_str(const gchar *option_name, return TRUE; } +static OsinfoOs * +find_os(const gchar *os_str) +{ + OsinfoDb *db = get_default_osinfo_db(); + OsinfoOs *ret = NULL; + + if (!db) + return NULL; + + ret = osinfo_db_get_os(db, os_str); + + return ret; +} + +static OsinfoOs * +find_os_by_short_id(const char *short_id) +{ + OsinfoDb *db = get_default_osinfo_db(); + OsinfoOs *ret = NULL; + OsinfoOsList *oses = NULL; + GList *list, *list_iterator; + + if (!db) + return NULL; + + oses = osinfo_db_get_os_list(db); + + if (!oses) + goto cleanup; + + list = osinfo_list_get_elements(OSINFO_LIST(oses)); + for (list_iterator = list; list_iterator; list_iterator = list_iterator->next) { + const char *id = osinfo_entity_get_param_value(list_iterator->data, + OSINFO_PRODUCT_PROP_SHORT_ID); + + if (id && g_str_equal(id, short_id)) { + ret = OSINFO_OS(list_iterator->data); + g_object_ref(ret); + break; + } + } + + g_object_unref(oses); + +cleanup: + return ret; +} + +static OsinfoOs * +guess_os_from_disk(GList *disk_list) +{ + OsinfoOs *ret = NULL; + GList *tmp = g_list_first(disk_list); + OsinfoLoader *loader = osinfo_loader_new(); + OsinfoDb *db; + + osinfo_loader_process_default_path(loader, NULL); + db = osinfo_loader_get_db(loader); + + while (tmp) { + char *path = (char *) tmp->data; + char *sep = strchr(path, ','); + OsinfoMedia *media = NULL; + OsinfoMedia *matched_media = NULL; + + if (sep) { + path = g_strndup(path, sep-path); + } + + media = osinfo_media_create_from_location(path, NULL, NULL); + if (!media) { + tmp = g_list_next(tmp); + continue; + } + + ret = osinfo_db_guess_os_from_media(db, media, &matched_media); + + if (sep) + g_free(path); + + if (ret) { + g_object_ref(ret); + break; + } + + tmp = g_list_next(tmp); + } + + g_clear_object(&loader); + return ret; +} + #define CHECK_ERROR \ if (error) { \ print_error("%s", error->message); \ @@ -347,10 +439,6 @@ main(int argc, char *argv[]) g_print ("option parsing failed: %s\n", error->message); return EXIT_FAILURE; } - if (!os_str) { - print_error("Operating system was not specified"); - return EXIT_FAILURE; - } if (!platform_str) { print_error("Platform was not specified"); return EXIT_FAILURE; @@ -363,9 +451,21 @@ main(int argc, char *argv[]) caps = gvir_connection_get_capabilities(conn, &error); CHECK_ERROR; - os = osinfo_os_new(os_str); platform = osinfo_platform_new(platform_str); + if (os_str) { + os = find_os(os_str); + if (!os) + os = find_os_by_short_id(os_str); + } else { + os = guess_os_from_disk(disk_str_list); + } + + if (!os) { + print_error("Operating system could not be find or guessed"); + goto cleanup; + } + domain = gvir_designer_domain_new(os, platform, caps); gvir_designer_domain_setup_machine(domain, &error); -- 1.7.8.6

Hey, Looks good, a few cosmetic comments below, On Thu, Sep 13, 2012 at 04:04:29PM +0200, Michal Privoznik wrote:
In some cases telling OS version is redundant as ISO image with specified OS is passed some arguments later as disk. Don't require --os then. --- examples/virtxml.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 105 insertions(+), 5 deletions(-)
diff --git a/examples/virtxml.c b/examples/virtxml.c index b0c3a77..4ec9cd8 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -293,6 +293,98 @@ add_iface_str(const gchar *option_name, return TRUE; }
+static OsinfoOs * +find_os(const gchar *os_str) +{ + OsinfoDb *db = get_default_osinfo_db(); + OsinfoOs *ret = NULL; + + if (!db) + return NULL; + + ret = osinfo_db_get_os(db, os_str); + + return ret; +} + +static OsinfoOs * +find_os_by_short_id(const char *short_id) +{ + OsinfoDb *db = get_default_osinfo_db(); + OsinfoOs *ret = NULL; + OsinfoOsList *oses = NULL; + GList *list, *list_iterator;
I generally go with the much shorter 'it' instead of list_iterator, no need to change if you prefer the more explicit version
+ + if (!db) + return NULL; + + oses = osinfo_db_get_os_list(db); + + if (!oses) + goto cleanup; + + list = osinfo_list_get_elements(OSINFO_LIST(oses)); + for (list_iterator = list; list_iterator; list_iterator = list_iterator->next) { + const char *id = osinfo_entity_get_param_value(list_iterator->data, + OSINFO_PRODUCT_PROP_SHORT_ID); + + if (id && g_str_equal(id, short_id)) {
you can use g_strcmp0 here to avoid testing 'id'.
+ ret = OSINFO_OS(list_iterator->data); + g_object_ref(ret); + break; + } + } + + g_object_unref(oses); + +cleanup: + return ret; +} + +static OsinfoOs * +guess_os_from_disk(GList *disk_list) +{ + OsinfoOs *ret = NULL; + GList *tmp = g_list_first(disk_list); + OsinfoLoader *loader = osinfo_loader_new(); + OsinfoDb *db; + + osinfo_loader_process_default_path(loader, NULL); + db = osinfo_loader_get_db(loader); + + while (tmp) {
I much prefer the for loop you use for a similar purpose in find_os_by_short_id, especially with the use of 'continue' in this loop.
+ char *path = (char *) tmp->data; + char *sep = strchr(path, ','); + OsinfoMedia *media = NULL; + OsinfoMedia *matched_media = NULL; + + if (sep) { + path = g_strndup(path, sep-path); + }
the {} with the 1 line block are inconsistent with what is done in the rest of the patch. Christophe

as in nearly all cases users will install the guest on current libvirt we've just obtained capabilities from. --- configure.ac | 2 +- examples/virtxml.c | 180 ++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 139 insertions(+), 43 deletions(-) diff --git a/configure.ac b/configure.ac index c214809..eb9c681 100644 --- a/configure.ac +++ b/configure.ac @@ -12,7 +12,7 @@ AM_SILENT_RULES([yes]) LIBOSINFO_REQUIRED=0.0.5 LIBVIRT_GCONFIG_REQUIRED=0.0.9 -LIBVIRT_GOBJECT_REQUIRED=0.0.8 +LIBVIRT_GOBJECT_REQUIRED=0.1.3 GOBJECT_INTROSPECTION_REQUIRED=0.10.8 LIBVIRT_DESIGNER_MAJOR_VERSION=`echo $VERSION | awk -F. '{print $1}'` diff --git a/examples/virtxml.c b/examples/virtxml.c index 4ec9cd8..cfe25ff 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -35,6 +35,8 @@ GList *disk_str_list = NULL; GList *iface_str_list = NULL; +OsinfoLoader *loader = NULL; +OsinfoDb *db = NULL; #define print_error(...) \ print_error_impl(__FUNCTION__, __LINE__, __VA_ARGS__) @@ -58,12 +60,11 @@ print_error_impl(const char *funcname, fprintf(stderr,"\n"); } -static OsinfoDb * -get_default_osinfo_db(void) +static gboolean +load_osinfo(void) { GError *err = NULL; - OsinfoLoader *loader = NULL; - OsinfoDb *ret = NULL; + gboolean ret = FALSE; loader = osinfo_loader_new(); osinfo_loader_process_default_path(loader, &err); @@ -72,8 +73,9 @@ get_default_osinfo_db(void) goto cleanup; } - ret = osinfo_loader_get_db(loader); - g_object_ref(ret); + db = osinfo_loader_get_db(loader); + g_object_ref(db); + ret = TRUE; cleanup: g_object_unref(loader); @@ -96,13 +98,12 @@ print_oses(const gchar *option_name, gpointer data, GError **error) { - OsinfoDb *db = get_default_osinfo_db(); OsinfoOsList *list = NULL; GList *oses = NULL; GList *os_iter; int ret = EXIT_FAILURE; - if (!db) + if (!db && !load_osinfo()) goto cleanup; printf(" Operating System ID\n" @@ -126,8 +127,6 @@ print_oses(const gchar *option_name, cleanup: if (list) g_object_unref(list); - if (db) - g_object_unref(db); exit(ret); return TRUE; @@ -139,13 +138,12 @@ print_platforms(const gchar *option_name, gpointer data, GError **error) { - OsinfoDb *db = get_default_osinfo_db(); OsinfoPlatformList *list = NULL; GList *platforms = NULL; GList *platform_iter; int ret = EXIT_FAILURE; - if (!db) + if (!db && !load_osinfo()) goto cleanup; printf(" Platform ID\n" @@ -169,8 +167,6 @@ print_platforms(const gchar *option_name, cleanup: if (list) g_object_unref(list); - if (db) - g_object_unref(db); exit(ret); return TRUE; @@ -293,13 +289,34 @@ add_iface_str(const gchar *option_name, return TRUE; } +static OsinfoEntity * +find_entity_by_short_id(OsinfoList *ent_list, + const char *short_id) +{ + OsinfoEntity *ret = NULL; + GList *list, *list_iterator; + + list = osinfo_list_get_elements(ent_list); + for (list_iterator = list; list_iterator; list_iterator = list_iterator->next) { + const char *id = osinfo_entity_get_param_value(list_iterator->data, + OSINFO_PRODUCT_PROP_SHORT_ID); + + if (id && g_str_equal(id, short_id)) { + ret = list_iterator->data; + g_object_ref(ret); + break; + } + } + + return ret; +} + static OsinfoOs * find_os(const gchar *os_str) { - OsinfoDb *db = get_default_osinfo_db(); OsinfoOs *ret = NULL; - if (!db) + if (!db && !load_osinfo()) return NULL; ret = osinfo_db_get_os(db, os_str); @@ -310,12 +327,11 @@ find_os(const gchar *os_str) static OsinfoOs * find_os_by_short_id(const char *short_id) { - OsinfoDb *db = get_default_osinfo_db(); OsinfoOs *ret = NULL; OsinfoOsList *oses = NULL; - GList *list, *list_iterator; + OsinfoEntity *found = NULL; - if (!db) + if (!db && !load_osinfo()) return NULL; oses = osinfo_db_get_os_list(db); @@ -323,21 +339,15 @@ find_os_by_short_id(const char *short_id) if (!oses) goto cleanup; - list = osinfo_list_get_elements(OSINFO_LIST(oses)); - for (list_iterator = list; list_iterator; list_iterator = list_iterator->next) { - const char *id = osinfo_entity_get_param_value(list_iterator->data, - OSINFO_PRODUCT_PROP_SHORT_ID); - - if (id && g_str_equal(id, short_id)) { - ret = OSINFO_OS(list_iterator->data); - g_object_ref(ret); - break; - } - } + found = find_entity_by_short_id(OSINFO_LIST(oses), short_id); - g_object_unref(oses); + if (!found) + goto cleanup; + ret = OSINFO_OS(found); cleanup: + if (oses) + g_object_unref(oses); return ret; } @@ -346,11 +356,9 @@ guess_os_from_disk(GList *disk_list) { OsinfoOs *ret = NULL; GList *tmp = g_list_first(disk_list); - OsinfoLoader *loader = osinfo_loader_new(); - OsinfoDb *db; - osinfo_loader_process_default_path(loader, NULL); - db = osinfo_loader_get_db(loader); + if (!db && !load_osinfo()) + return NULL; while (tmp) { char *path = (char *) tmp->data; @@ -381,7 +389,88 @@ guess_os_from_disk(GList *disk_list) tmp = g_list_next(tmp); } - g_clear_object(&loader); + return ret; +} + +static OsinfoPlatform * +find_platform(const char *platform_str) +{ + OsinfoPlatform *ret = NULL; + + if (!db && !load_osinfo()) + return NULL; + + ret = osinfo_db_get_platform(db, platform_str); + + return ret; +} + +static OsinfoPlatform * +find_platform_by_short_id(const char *short_id) +{ + OsinfoPlatform *ret = NULL; + OsinfoPlatformList *platforms = NULL; + OsinfoEntity *found = NULL; + + if (!db && !load_osinfo()) + goto cleanup; + + platforms = osinfo_db_get_platform_list(db); + + if (!platforms) + goto cleanup; + + found = find_entity_by_short_id(OSINFO_LIST(platforms), short_id); + + if (!found) + goto cleanup; + + ret = OSINFO_PLATFORM(found); + +cleanup: + if (platforms) + g_object_unref(platforms); + return ret; +} + +static OsinfoPlatform * +guess_platform_from_connect(GVirConnection *conn) +{ + OsinfoPlatform *ret = NULL; + gchar *hv_type = NULL; + gulong ver, major, minor, release; + char *short_id = NULL, *type = NULL; + + hv_type = gvir_connection_get_hypervisor_name(conn, NULL); + ver = gvir_connection_get_version(conn, NULL); + + if (!hv_type || !ver) { + print_error("Unable to get hypervisor and its version"); + exit(EXIT_FAILURE); + } + + /* do some mappings: + * QEMU -> kvm + * Xen -> xen + */ + type = g_ascii_strdown(hv_type, -1); + if (g_str_equal(type, "qemu")) { + g_free(type); + type = g_strdup("kvm"); + } + + major = ver / 1000000; + ver %= 1000000; + minor = ver / 1000; + release = ver % 1000 ; + + short_id = g_strdup_printf("%s-%lu.%lu.%lu", type, major, minor, release); + + ret = find_platform_by_short_id(short_id); + + g_free(short_id); + g_free(hv_type); + g_free(type); return ret; } @@ -439,10 +528,6 @@ main(int argc, char *argv[]) g_print ("option parsing failed: %s\n", error->message); return EXIT_FAILURE; } - if (!platform_str) { - print_error("Platform was not specified"); - return EXIT_FAILURE; - } conn = gvir_connection_new(connect_uri); gvir_connection_open(conn, NULL, &error); @@ -451,8 +536,6 @@ main(int argc, char *argv[]) caps = gvir_connection_get_capabilities(conn, &error); CHECK_ERROR; - platform = osinfo_platform_new(platform_str); - if (os_str) { os = find_os(os_str); if (!os) @@ -466,6 +549,19 @@ main(int argc, char *argv[]) goto cleanup; } + if (platform_str) { + platform = find_platform(platform_str); + if (!platform) + platform = find_platform_by_short_id(platform_str); + } else { + platform = guess_platform_from_connect(conn); + } + + if (!platform) { + print_error("Platform was not specified or could not be guessed"); + goto cleanup; + } + domain = gvir_designer_domain_new(os, platform, caps); gvir_designer_domain_setup_machine(domain, &error); -- 1.7.8.6

Looks good, ACK (would have been easier to review with the load_osinfo changes in a separate patch, this would have halved the size of the patch doing real code changes I think). Christophe On Thu, Sep 13, 2012 at 04:04:30PM +0200, Michal Privoznik wrote:
as in nearly all cases users will install the guest on current libvirt we've just obtained capabilities from. --- configure.ac | 2 +- examples/virtxml.c | 180 ++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 139 insertions(+), 43 deletions(-)
diff --git a/configure.ac b/configure.ac index c214809..eb9c681 100644 --- a/configure.ac +++ b/configure.ac @@ -12,7 +12,7 @@ AM_SILENT_RULES([yes])
LIBOSINFO_REQUIRED=0.0.5 LIBVIRT_GCONFIG_REQUIRED=0.0.9 -LIBVIRT_GOBJECT_REQUIRED=0.0.8 +LIBVIRT_GOBJECT_REQUIRED=0.1.3 GOBJECT_INTROSPECTION_REQUIRED=0.10.8
LIBVIRT_DESIGNER_MAJOR_VERSION=`echo $VERSION | awk -F. '{print $1}'` diff --git a/examples/virtxml.c b/examples/virtxml.c index 4ec9cd8..cfe25ff 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -35,6 +35,8 @@
GList *disk_str_list = NULL; GList *iface_str_list = NULL; +OsinfoLoader *loader = NULL; +OsinfoDb *db = NULL;
#define print_error(...) \ print_error_impl(__FUNCTION__, __LINE__, __VA_ARGS__) @@ -58,12 +60,11 @@ print_error_impl(const char *funcname, fprintf(stderr,"\n"); }
-static OsinfoDb * -get_default_osinfo_db(void) +static gboolean +load_osinfo(void) { GError *err = NULL; - OsinfoLoader *loader = NULL; - OsinfoDb *ret = NULL; + gboolean ret = FALSE;
loader = osinfo_loader_new(); osinfo_loader_process_default_path(loader, &err); @@ -72,8 +73,9 @@ get_default_osinfo_db(void) goto cleanup; }
- ret = osinfo_loader_get_db(loader); - g_object_ref(ret); + db = osinfo_loader_get_db(loader); + g_object_ref(db); + ret = TRUE;
cleanup: g_object_unref(loader); @@ -96,13 +98,12 @@ print_oses(const gchar *option_name, gpointer data, GError **error) { - OsinfoDb *db = get_default_osinfo_db(); OsinfoOsList *list = NULL; GList *oses = NULL; GList *os_iter; int ret = EXIT_FAILURE;
- if (!db) + if (!db && !load_osinfo()) goto cleanup;
printf(" Operating System ID\n" @@ -126,8 +127,6 @@ print_oses(const gchar *option_name, cleanup: if (list) g_object_unref(list); - if (db) - g_object_unref(db);
exit(ret); return TRUE; @@ -139,13 +138,12 @@ print_platforms(const gchar *option_name, gpointer data, GError **error) { - OsinfoDb *db = get_default_osinfo_db(); OsinfoPlatformList *list = NULL; GList *platforms = NULL; GList *platform_iter; int ret = EXIT_FAILURE;
- if (!db) + if (!db && !load_osinfo()) goto cleanup;
printf(" Platform ID\n" @@ -169,8 +167,6 @@ print_platforms(const gchar *option_name, cleanup: if (list) g_object_unref(list); - if (db) - g_object_unref(db);
exit(ret); return TRUE; @@ -293,13 +289,34 @@ add_iface_str(const gchar *option_name, return TRUE; }
+static OsinfoEntity * +find_entity_by_short_id(OsinfoList *ent_list, + const char *short_id) +{ + OsinfoEntity *ret = NULL; + GList *list, *list_iterator; + + list = osinfo_list_get_elements(ent_list); + for (list_iterator = list; list_iterator; list_iterator = list_iterator->next) { + const char *id = osinfo_entity_get_param_value(list_iterator->data, + OSINFO_PRODUCT_PROP_SHORT_ID); + + if (id && g_str_equal(id, short_id)) { + ret = list_iterator->data; + g_object_ref(ret); + break; + } + } + + return ret; +} + static OsinfoOs * find_os(const gchar *os_str) { - OsinfoDb *db = get_default_osinfo_db(); OsinfoOs *ret = NULL;
- if (!db) + if (!db && !load_osinfo()) return NULL;
ret = osinfo_db_get_os(db, os_str); @@ -310,12 +327,11 @@ find_os(const gchar *os_str) static OsinfoOs * find_os_by_short_id(const char *short_id) { - OsinfoDb *db = get_default_osinfo_db(); OsinfoOs *ret = NULL; OsinfoOsList *oses = NULL; - GList *list, *list_iterator; + OsinfoEntity *found = NULL;
- if (!db) + if (!db && !load_osinfo()) return NULL;
oses = osinfo_db_get_os_list(db); @@ -323,21 +339,15 @@ find_os_by_short_id(const char *short_id) if (!oses) goto cleanup;
- list = osinfo_list_get_elements(OSINFO_LIST(oses)); - for (list_iterator = list; list_iterator; list_iterator = list_iterator->next) { - const char *id = osinfo_entity_get_param_value(list_iterator->data, - OSINFO_PRODUCT_PROP_SHORT_ID); - - if (id && g_str_equal(id, short_id)) { - ret = OSINFO_OS(list_iterator->data); - g_object_ref(ret); - break; - } - } + found = find_entity_by_short_id(OSINFO_LIST(oses), short_id);
- g_object_unref(oses); + if (!found) + goto cleanup; + ret = OSINFO_OS(found);
cleanup: + if (oses) + g_object_unref(oses); return ret; }
@@ -346,11 +356,9 @@ guess_os_from_disk(GList *disk_list) { OsinfoOs *ret = NULL; GList *tmp = g_list_first(disk_list); - OsinfoLoader *loader = osinfo_loader_new(); - OsinfoDb *db;
- osinfo_loader_process_default_path(loader, NULL); - db = osinfo_loader_get_db(loader); + if (!db && !load_osinfo()) + return NULL;
while (tmp) { char *path = (char *) tmp->data; @@ -381,7 +389,88 @@ guess_os_from_disk(GList *disk_list) tmp = g_list_next(tmp); }
- g_clear_object(&loader); + return ret; +} + +static OsinfoPlatform * +find_platform(const char *platform_str) +{ + OsinfoPlatform *ret = NULL; + + if (!db && !load_osinfo()) + return NULL; + + ret = osinfo_db_get_platform(db, platform_str); + + return ret; +} + +static OsinfoPlatform * +find_platform_by_short_id(const char *short_id) +{ + OsinfoPlatform *ret = NULL; + OsinfoPlatformList *platforms = NULL; + OsinfoEntity *found = NULL; + + if (!db && !load_osinfo()) + goto cleanup; + + platforms = osinfo_db_get_platform_list(db); + + if (!platforms) + goto cleanup; + + found = find_entity_by_short_id(OSINFO_LIST(platforms), short_id); + + if (!found) + goto cleanup; + + ret = OSINFO_PLATFORM(found); + +cleanup: + if (platforms) + g_object_unref(platforms); + return ret; +} + +static OsinfoPlatform * +guess_platform_from_connect(GVirConnection *conn) +{ + OsinfoPlatform *ret = NULL; + gchar *hv_type = NULL; + gulong ver, major, minor, release; + char *short_id = NULL, *type = NULL; + + hv_type = gvir_connection_get_hypervisor_name(conn, NULL); + ver = gvir_connection_get_version(conn, NULL); + + if (!hv_type || !ver) { + print_error("Unable to get hypervisor and its version"); + exit(EXIT_FAILURE); + } + + /* do some mappings: + * QEMU -> kvm + * Xen -> xen + */ + type = g_ascii_strdown(hv_type, -1); + if (g_str_equal(type, "qemu")) { + g_free(type); + type = g_strdup("kvm"); + } + + major = ver / 1000000; + ver %= 1000000; + minor = ver / 1000; + release = ver % 1000 ; + + short_id = g_strdup_printf("%s-%lu.%lu.%lu", type, major, minor, release); + + ret = find_platform_by_short_id(short_id); + + g_free(short_id); + g_free(hv_type); + g_free(type); return ret; }
@@ -439,10 +528,6 @@ main(int argc, char *argv[]) g_print ("option parsing failed: %s\n", error->message); return EXIT_FAILURE; } - if (!platform_str) { - print_error("Platform was not specified"); - return EXIT_FAILURE; - }
conn = gvir_connection_new(connect_uri); gvir_connection_open(conn, NULL, &error); @@ -451,8 +536,6 @@ main(int argc, char *argv[]) caps = gvir_connection_get_capabilities(conn, &error); CHECK_ERROR;
- platform = osinfo_platform_new(platform_str); - if (os_str) { os = find_os(os_str); if (!os) @@ -466,6 +549,19 @@ main(int argc, char *argv[]) goto cleanup; }
+ if (platform_str) { + platform = find_platform(platform_str); + if (!platform) + platform = find_platform_by_short_id(platform_str); + } else { + platform = guess_platform_from_connect(conn); + } + + if (!platform) { + print_error("Platform was not specified or could not be guessed"); + goto cleanup; + } + domain = gvir_designer_domain_new(os, platform, caps);
gvir_designer_domain_setup_machine(domain, &error); -- 1.7.8.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Users can choose between minimum and recommended values for VCPU count and amount of RAM. Moreover, recommended values should inherit defaults from the minimum. --- examples/virtxml.c | 12 ++++ libvirt-designer/libvirt-designer-domain.c | 91 ++++++++++++++++++++++++++++ libvirt-designer/libvirt-designer-domain.h | 8 +++ libvirt-designer/libvirt-designer.sym | 1 + 4 files changed, 112 insertions(+), 0 deletions(-) diff --git a/examples/virtxml.c b/examples/virtxml.c index cfe25ff..2908667 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -496,6 +496,7 @@ main(int argc, char *argv[]) static char *platform_str = NULL; static char *arch_str = NULL; static char *connect_uri = NULL; + static char *resources = NULL; GOptionContext *context; static GOptionEntry entries[] = @@ -516,6 +517,8 @@ main(int argc, char *argv[]) "add disk to domain with PATH being source and FORMAT its format", "PATH[,FORMAT]"}, {"interface", 'i', 0, G_OPTION_ARG_CALLBACK, add_iface_str, "add interface with NETWORK source. Possible ARGs: mac, link={up,down}", "NETWORK[,ARG=VAL]"}, + {"resources", 'r', 0, G_OPTION_ARG_STRING, &resources, + "Set minimal or recommended values for cpu count and RAM amount", "{minimal|recommended}"}, {NULL} }; @@ -572,6 +575,15 @@ main(int argc, char *argv[]) CHECK_ERROR; } + if (resources) { + gvir_designer_domain_setup_resources(domain, resources, &error); + CHECK_ERROR; + } else { + gvir_designer_domain_setup_resources(domain, + GVIR_DESIGNER_DOMAIN_RESOURCES_RECOMMENDED, + NULL); + } + g_list_foreach(disk_str_list, add_disk, domain); g_list_foreach(iface_str_list, add_iface, domain); diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 194fdf5..de4582c 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -1033,3 +1033,94 @@ gvir_designer_domain_add_interface_network(GVirDesignerDomain *design, return ret; } + + +static void +gvir_designer_domain_get_resources(OsinfoResourcesList *res_list, + const gchar *design_arch, + gint *design_n_cpus, + gint64 *design_ram) +{ + GList *elem_list, *elem_iterator; + + if (!res_list) + return; + + elem_list = osinfo_list_get_elements(OSINFO_LIST(res_list)); + for (elem_iterator = elem_list; elem_iterator; elem_iterator = elem_iterator->next) { + OsinfoResources *res = OSINFO_RESOURCES(elem_iterator->data); + const char *arch = osinfo_resources_get_architecture(res); + gint n_cpus = -1; + gint64 ram = -1; + + if (g_str_equal(arch, "all") || + g_str_equal(arch, design_arch)) { + n_cpus = osinfo_resources_get_n_cpus(res); + ram = osinfo_resources_get_ram(res); + if (n_cpus > 0) { + *design_n_cpus = n_cpus; + } + if (ram > 0) { + /* libosinfo returns RAM in B, libvirt-gconfig takes it in KB */ + *design_ram = ram / 1024; + } + break; + } + } +} + + +/** + * gvir_designer_domain_setup_resources: + * @design: (transfer none): the domain designer intance + * @req: (transfer none): requirements to set + * + * Set minimal or recommended resources on @design. + * + * Returns: (transfer none): TRUE when successfully set, FALSE otherwise. + */ +gboolean gvir_designer_domain_setup_resources(GVirDesignerDomain *design, + const gchar *req, + GError **error) +{ + g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), FALSE); + gboolean ret = FALSE; + OsinfoResourcesList *res_list_min = NULL, *res_list_rec = NULL; + GVirConfigDomainOs *os = gvir_config_domain_get_os(design->priv->config); + const gchar *design_arch = os ? gvir_config_domain_os_get_arch(os) : ""; + gint n_cpus = -1; + gint64 ram = -1; + + /* If user request recommended settings those may just override + * only a few settings when compared to minimal. So we must implement + * inheritance here. */ + res_list_min = osinfo_os_get_minimum_resources(design->priv->os); + res_list_rec = osinfo_os_get_recommended_resources(design->priv->os); + if (g_str_equal(req, GVIR_DESIGNER_DOMAIN_RESOURCES_MINIMAL)) { + gvir_designer_domain_get_resources(res_list_min, design_arch, + &n_cpus, &ram); + } else if (g_str_equal(req, GVIR_DESIGNER_DOMAIN_RESOURCES_RECOMMENDED)) { + gvir_designer_domain_get_resources(res_list_min, design_arch, + &n_cpus, &ram); + gvir_designer_domain_get_resources(res_list_rec, design_arch, + &n_cpus, &ram); + } else { + g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0, + "Unknown resources argument: '%s'", req); + goto cleanup; + } + + if ((n_cpus <= 0) && (ram <= 0)) { + g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0, + "Unable to find resources in libosinfo database"); + goto cleanup; + } + + if (n_cpus > 0) + gvir_config_domain_set_vcpus(design->priv->config, n_cpus); + if (ram > 0) + gvir_config_domain_set_memory(design->priv->config, ram); + +cleanup: + return ret; +} diff --git a/libvirt-designer/libvirt-designer-domain.h b/libvirt-designer/libvirt-designer-domain.h index 5097393..28a1d06 100644 --- a/libvirt-designer/libvirt-designer-domain.h +++ b/libvirt-designer/libvirt-designer-domain.h @@ -39,6 +39,9 @@ G_BEGIN_DECLS #define GVIR_DESIGNER_IS_DOMAIN_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_DESIGNER_TYPE_DOMAIN)) #define GVIR_DESIGNER_DOMAIN_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_DESIGNER_TYPE_DOMAIN, GVirDesignerDomainClass)) +#define GVIR_DESIGNER_DOMAIN_RESOURCES_MINIMAL "minimal" +#define GVIR_DESIGNER_DOMAIN_RESOURCES_RECOMMENDED "recommended" + typedef struct _GVirDesignerDomain GVirDesignerDomain; typedef struct _GVirDesignerDomainPrivate GVirDesignerDomainPrivate; typedef struct _GVirDesignerDomainClass GVirDesignerDomainClass; @@ -104,6 +107,11 @@ GVirConfigDomainDisk *gvir_designer_domain_add_disk_device(GVirDesignerDomain *d GVirConfigDomainInterface *gvir_designer_domain_add_interface_network(GVirDesignerDomain *design, const char *network, GError **error); + +gboolean gvir_designer_domain_setup_resources(GVirDesignerDomain *design, + const gchar *req, + 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 317a07b..0dde9a6 100644 --- a/libvirt-designer/libvirt-designer.sym +++ b/libvirt-designer/libvirt-designer.sym @@ -13,6 +13,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_resources; gvir_designer_domain_setup_machine; gvir_designer_domain_setup_machine_full; -- 1.7.8.6

On Thu, Sep 13, 2012 at 04:04:31PM +0200, Michal Privoznik wrote:
Users can choose between minimum and recommended values for VCPU count and amount of RAM. Moreover, recommended values should inherit defaults from the minimum. --- examples/virtxml.c | 12 ++++ libvirt-designer/libvirt-designer-domain.c | 91 ++++++++++++++++++++++++++++ libvirt-designer/libvirt-designer-domain.h | 8 +++ libvirt-designer/libvirt-designer.sym | 1 + 4 files changed, 112 insertions(+), 0 deletions(-)
diff --git a/examples/virtxml.c b/examples/virtxml.c index cfe25ff..2908667 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -496,6 +496,7 @@ main(int argc, char *argv[]) static char *platform_str = NULL; static char *arch_str = NULL; static char *connect_uri = NULL; + static char *resources = NULL; GOptionContext *context;
static GOptionEntry entries[] = @@ -516,6 +517,8 @@ main(int argc, char *argv[]) "add disk to domain with PATH being source and FORMAT its format", "PATH[,FORMAT]"}, {"interface", 'i', 0, G_OPTION_ARG_CALLBACK, add_iface_str, "add interface with NETWORK source. Possible ARGs: mac, link={up,down}", "NETWORK[,ARG=VAL]"}, + {"resources", 'r', 0, G_OPTION_ARG_STRING, &resources, + "Set minimal or recommended values for cpu count and RAM amount", "{minimal|recommended}"}, {NULL} };
@@ -572,6 +575,15 @@ main(int argc, char *argv[]) CHECK_ERROR; }
+ if (resources) { + gvir_designer_domain_setup_resources(domain, resources, &error); + CHECK_ERROR; + } else { + gvir_designer_domain_setup_resources(domain, + GVIR_DESIGNER_DOMAIN_RESOURCES_RECOMMENDED, + NULL); + } + g_list_foreach(disk_str_list, add_disk, domain);
g_list_foreach(iface_str_list, add_iface, domain); diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 194fdf5..de4582c 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -1033,3 +1033,94 @@ gvir_designer_domain_add_interface_network(GVirDesignerDomain *design,
return ret; } + + +static void +gvir_designer_domain_get_resources(OsinfoResourcesList *res_list, + const gchar *design_arch, + gint *design_n_cpus, + gint64 *design_ram) +{ + GList *elem_list, *elem_iterator; + + if (!res_list) + return; + + elem_list = osinfo_list_get_elements(OSINFO_LIST(res_list)); + for (elem_iterator = elem_list; elem_iterator; elem_iterator = elem_iterator->next) { + OsinfoResources *res = OSINFO_RESOURCES(elem_iterator->data); + const char *arch = osinfo_resources_get_architecture(res); + gint n_cpus = -1; + gint64 ram = -1; + + if (g_str_equal(arch, "all") || + g_str_equal(arch, design_arch)) { + n_cpus = osinfo_resources_get_n_cpus(res); + ram = osinfo_resources_get_ram(res);
extra space after '='
+ if (n_cpus > 0) { + *design_n_cpus = n_cpus; + } + if (ram > 0) { + /* libosinfo returns RAM in B, libvirt-gconfig takes it in KB */ + *design_ram = ram / 1024; + } + break; + } + } +} + + +/** + * gvir_designer_domain_setup_resources: + * @design: (transfer none): the domain designer intance
instance
+ * @req: (transfer none): requirements to set + * + * Set minimal or recommended resources on @design. + * + * Returns: (transfer none): TRUE when successfully set, FALSE otherwise. + */ +gboolean gvir_designer_domain_setup_resources(GVirDesignerDomain *design, + const gchar *req,
I think an enum would be better than a string based API. If we go with a string, can you explicitly mention the valid values in the api doc? Christophe
participants (2)
-
Christophe Fergeau
-
Michal Privoznik