[libvirt] [libvirt-designer PATCHv2 3/3] Rework disk bus type handling
Michal Privoznik
mprivozn at redhat.com
Fri Apr 19 09:46:21 UTC 2013
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
More information about the libvir-list
mailing list