[virt-tools-list] [virt-manager PATCH] virtManager/addhardware: get supported disk bus types from libvirt

[Pavel Hrdina]

On Sun, Feb 05, 2017 at 04:51:57PM -0500, Cole Robinson wrote:
> On 01/20/2017 10:21 AM, Pavel Hrdina wrote:
> > Libvirt provides domain capabilities where supported disk bus types are
> > listed.  Virt-manager should try to get those bus types.  The old code
> > remains as fallback if domain capabilities doesn't contain the disk
> > bus types.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1387218
> > 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> >  virtManager/addhardware.py | 49 +++++++++++++++++++++++++++++-----------------
> >  1 file changed, 31 insertions(+), 18 deletions(-)
> > 
> > diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
> > index aab90957..e63ad3d0 100644
> > --- a/virtManager/addhardware.py
> > +++ b/virtManager/addhardware.py
> > @@ -690,25 +690,38 @@ class vmmAddHardware(vmmGObjectUI):
> >  
> >      @staticmethod
> >      def populate_disk_bus_combo(vm, devtype, model):
> > +        # try to get supported disk bus types from domain capabilities
> > +        domcaps = vm.get_domain_capabilities()
> > +        disk_bus_types = None
> > +        if "bus" in domcaps.devices.disk.enum_names():
> > +            disk_bus_types = domcaps.devices.disk.get_enum("bus").get_values()
> > +
> 
> Unfortunately libvirt's domcaps don't seem to be exhaustive. For example,
> bus=sd isn't listed, which virt-manager supports in the existing code. So in
> order to enable this we would need to 1) fix libvirt, 2) only use the domcaps
> code from known good versions. That's kind of the general difficulty in
> switching to these types of capabilities reporting APIs unfortunately...

Yes, I've noticed that, but unfortunately after I've pushed the patch.  I would
say that this makes things better than worst even though virt-manager will not
display all available buses for disks.  I agree that Libvirt should be fixed
to provide correct data if it already provides these data.

Pavel

> 
> Thanks,
> Cole
> 
> > +        # if there are no disk bus types in domain capabilities fallback to
> > +        # old code
> > +        if not disk_bus_types:
> > +            disk_bus_types = []
> > +            if vm.is_hvm():
> > +                if not vm.get_xmlobj().os.is_q35():
> > +                    disk_bus_types.append("ide")
> > +                disk_bus_types.append("sata")
> > +                disk_bus_types.append("fdc")
> > +
> > +                if not vm.stable_defaults():
> > +                    disk_bus_types.append("scsi")
> > +                    disk_bus_types.append("usb")
> > +
> > +            if vm.get_hv_type() in ["qemu", "kvm", "test"]:
> > +                disk_bus_types.append("sd")
> > +                disk_bus_types.append("virtio")
> > +                if "scsi" not in disk_bus_types:
> > +                    disk_bus_types.append("scsi")
> > +
> > +            if vm.conn.is_xen() or vm.conn.is_test_conn():
> > +                disk_bus_types.append("xen")
> > +
> >          rows = []
> > -        if vm.is_hvm():
> > -            if not vm.get_xmlobj().os.is_q35():
> > -                rows.append(["ide", "IDE"])
> > -            rows.append(["sata", "SATA"])
> > -            rows.append(["fdc", _("Floppy")])
> > -
> > -            if not vm.stable_defaults():
> > -                rows.append(["scsi", "SCSI"])
> > -                rows.append(["usb", "USB"])
> > -
> > -        if vm.get_hv_type() in ["qemu", "kvm", "test"]:
> > -            rows.append(["sd", "SD"])
> > -            rows.append(["virtio", "VirtIO"])
> > -            if not rows.count(["scsi", "SCSI"]):
> > -                rows.append(["scsi", "SCSI"])
> > -
> > -        if vm.conn.is_xen() or vm.conn.is_test_conn():
> > -            rows.append(["xen", "Xen"])
> > +        for bus in disk_bus_types:
> > +            rows.append([bus, virtinst.VirtualDisk.pretty_disk_bus(bus)])
> >  
> >          model.clear()
> >  
> > 
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20170206/bc208b27/attachment.sig>