On 18.04.2013 18:08, Christophe Fergeau wrote: > The current handling of bus types has some issues: > - it assumes that if the design uses a disk controller hanging off > a PCI bus, then it can use virtio, which is not true for > Windows for example unless an additional driver is installed > - it checks for "ide", "sata", "virtio" bus names, but they are not > used in libosinfo, and "sata is not mentioned in libosinfo.rng > - if the bus type could not determined, falling back to an IDE > bus should be a safe guess > > This commit changes the code to guessing the best disk controller > to use, and then derives the bus type from it when needed. > One limitation of this approach is that we are currently limited to > virtio or IDE as libosinfo is not expressive enough for us to tell > if a given disk controller is IDE/SATA/SCSI/... > One way of making this distinction possible would be to attach the > PCI subclass to libosinfo device descriptions as this contains the > information we need. > --- > libvirt-designer/libvirt-designer-domain.c | 185 ++++++++++++++--------------- > 1 file changed, 91 insertions(+), 94 deletions(-) > > diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c > index f959215..01e48ae 100644 > --- a/libvirt-designer/libvirt-designer-domain.c > +++ b/libvirt-designer/libvirt-designer-domain.c > @@ -23,6 +23,7 @@ > */ > > #include <config.h> > +#include <string.h> This shouldn't be needed [1] > #include <sys/utsname.h> > > #include "libvirt-designer/libvirt-designer.h" > @@ -68,6 +69,8 @@ static gboolean error_is_set(GError **error) > return ((error != NULL) && (*error != NULL)); > } > > +static const char GVIR_DESIGNER_VIRTIO_BLOCK_DEVICE_ID[] = "http://pciids.sourceforge.net/v2.2/pci.ids/1af4/1001";; > + > enum { > PROP_0, > PROP_CONFIG, > @@ -925,6 +869,84 @@ gvir_designer_domain_next_disk_target(GVirDesignerDomain *design, > return ret; > } > > +static OsinfoDevice * > +gvir_designer_domain_get_preferred_disk_controller(GVirDesignerDomain *design, > + GError **error) > +{ > + OsinfoDevice *dev = NULL; > + OsinfoDeviceLink *dev_link = gvir_designer_domain_get_preferred_device(design, > + "block", > + error); > + if (dev_link == NULL) > + goto cleanup; > + > + dev = osinfo_devicelink_get_target(dev_link); > + > +cleanup: > + if (dev_link != NULL) > + g_object_unref(dev_link); > + return dev; > +} > + > + > +static OsinfoDevice * > +gvir_designer_domain_get_fallback_disk_controller(GVirDesignerDomain *design, > + GError **error) > +{ > + OsinfoEntity *dev = NULL; > + OsinfoDeviceList *devices; > + OsinfoFilter *filter; > + int virt_type; > + > + filter = osinfo_filter_new(); > + osinfo_filter_add_constraint(filter, OSINFO_DEVICE_PROP_CLASS, "block"); > + devices = gvir_designer_domain_get_supported_devices(design, filter); > + g_object_unref(G_OBJECT(filter)); > + > + if ((devices == NULL) || > + (osinfo_list_get_length(OSINFO_LIST(devices)) == 0)) { No need for enclosing these two conditions in parentheses here ... > + goto cleanup; > + } > + > + virt_type = gvir_config_domain_get_virt_type(design->priv->config); > + if ((virt_type == GVIR_CONFIG_DOMAIN_VIRT_QEMU) || > + (virt_type == GVIR_CONFIG_DOMAIN_VIRT_KQEMU) || > + (virt_type == GVIR_CONFIG_DOMAIN_VIRT_KVM)) { ... here ... > + /* If using QEMU; we favour using virtio-block */ > + OsinfoList *tmp_devices; > + filter = osinfo_filter_new(); > + osinfo_filter_add_constraint(filter, > + OSINFO_ENTITY_PROP_ID, > + GVIR_DESIGNER_VIRTIO_BLOCK_DEVICE_ID); > + tmp_devices = osinfo_list_new_filtered(OSINFO_LIST(devices), filter); > + if ((tmp_devices != NULL) && > + (osinfo_list_get_length(OSINFO_LIST(tmp_devices)) > 0)) { ... and here. > + g_object_unref(devices); > + devices = OSINFO_DEVICELIST(tmp_devices); > + } > + g_object_unref(G_OBJECT(filter)); > + } > + > + dev = osinfo_list_get_nth(OSINFO_LIST(devices), 0); > + g_object_ref(G_OBJECT(dev)); > + g_object_unref(G_OBJECT(devices)); > + > +cleanup: > + return OSINFO_DEVICE(dev); > +} > + > +static GVirConfigDomainDiskBus > +gvir_designer_domain_get_bus_type_from_controller(GVirDesignerDomain *design, > + OsinfoDevice *controller) > +{ > + const char *id; > + > + id = osinfo_entity_get_id(OSINFO_ENTITY(controller)); > + if (strcmp(id, GVIR_DESIGNER_VIRTIO_BLOCK_DEVICE_ID) == 0) 1: as g_str_equal is preferred over strcmp. > + return GVIR_CONFIG_DOMAIN_DISK_BUS_VIRTIO; > + > + return GVIR_CONFIG_DOMAIN_DISK_BUS_IDE; > +} > ACK if you address nits pointed out. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list