On Mon, Sep 10, 2012 at 03:58:26PM +0200, Michal Privoznik wrote:
+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);
+ if (!dev_list)
+ goto cleanup;
+
+ 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);
+ if (ret)
+ ret = g_list_copy(ret);
NULL is a valid list (empty list), so g_list_copy(NULL); is fine.
+
+cleanup:
+ 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");
g_set_error_literal would be slightly better here.
+ 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);
+ if (dev)
+ 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) {
+ if (!*error)
I haven't looked at the callers, but in public APIs, passing a NULL
GError** is acceptable, so this test would be better as 'if (error != NULL
&& *error != NULL)' (I always wonder why glib does not provide
g_error_is_set).
+ g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0,
+ "Unable to find any disk bus type");
+ goto error;
+ }
+
+ item = g_list_first(bus_str_list);
+ bus_str = item->data;
+ if (!bus_str)
+ goto error;
Looks like 'error' will be leaked in we go to error.
Christophe