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
Attachment:
pgpsvfEPasHHE.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list