Fix rendering of VMware floppy drive #5041
Conversation
| @@ -55,6 +55,8 @@ def calculate_disk_name(disk) | |||
| "Hard Disk (SCSI #{disk.location})" | |||
| when "scsi-passthru" | |||
| "Generic SCSI (#{disk.location})" | |||
| when "floppy" | |||
| "Floppy Drive (SIO #{disk.location})" | |||
There was a problem hiding this comment.
Could you use (#{disk.controller_type} #{disk.location}) for all of these? Hard-coding Hard Disk (SCSI isn't correct since you can have disks on e.g. IDE
There was a problem hiding this comment.
You don't want to have SIO, SCSI, etc. hardcoded for floppy drives only, or for all disk types in general?
There was a problem hiding this comment.
So for consistency with the rest of this I'm fine with this, but I think we should revisit this and use the disk.controller_type for the rest of these.
There was a problem hiding this comment.
# select distinct(controller_type) from disks order by controller_type;
controller_type
------------------
amazon
azure
google
ide
IDE
openstack
OpenStack Volume
scsi
sio
virtio
virtio_scsi
(11 rows)
These are all controller types that I have in my devel DB. Does that show all possible types? Why is there both ide and IDE? I'm trying to figure out what would things look like in UI once we take it directly from DB.
There was a problem hiding this comment.
For amazon I also see "EBS Volume", great question on ide vs IDE my guess is that vmware has lowercase scsi, sio, ide and it looks like scvmm is the one adding IDE.
I'm not sure why the cloud providers were just putting the name in the controller_type, that doesn't make a whole lot of sense :) I'd rather nothing if it isn't something like scsi/ide/virtio ¯_(ツ)_/¯
There was a problem hiding this comment.
@agrare Besides what we're discussing here (probably a material for different PR), do you approve changes in this PR? Thanks.
|
Checked commit mzazrivec@114a262 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
Before:
After:
@agrare Would this be correct fix for the linked bz? Thanks.
https://bugzilla.redhat.com/show_bug.cgi?id=1650501