When we change disk bus to 'scsi' or add scsi disks, virt-manager relies on libvirt's rules to create addresses. Sometimes it works, sometimes not, Espectially in multiple scsi controllers case, we can not exactly control which virtual disks attach to which scsi controller, sometimes libvirt creates unnecessary scsi controllers for us, attach virtual disks to the scsi controllers which we don't want. It has confused users for a while. This patch added two new option to solve the above issue: 1. add virtio-scsi bus option. users can decide which kind of scsi bus they want to use, instead of guessing through libvirt. 2. add controller index option. when users configure disk bus, it allows users to exactly choice which existed scsi controller they want to attach or create a new one. The patch changed some mechanism about adding/configuring scsi bus, and generating device addrStr instead of libvirt did, it avoids some annoying issues. Signed-off-by: Lin Ma <lma@xxxxxxxx> --- ui/addhardware.ui | 40 +++++++++++++++ ui/details.ui | 40 +++++++++++++++ virtManager/addhardware.py | 125 +++++++++++++++++++++++++++++++++++---------- virtManager/details.py | 80 ++++++++++++++++++++++++++++- virtManager/domain.py | 7 ++- virtinst/device.py | 15 +++++- virtinst/devicedisk.py | 2 + 7 files changed, 278 insertions(+), 31 deletions(-) diff --git a/ui/addhardware.ui b/ui/addhardware.ui index eb476dab..e90d31e6 100644 --- a/ui/addhardware.ui +++ b/ui/addhardware.ui @@ -169,6 +169,45 @@ <property name="top_attach">0</property> </packing> </child> + <child> + <object class="GtkGrid" id="storage-scsi-idx"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="row_spacing">3</property> + <property name="column_spacing">8</property> + <child> + <object class="GtkLabel" id="label37"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="halign">start</property> + <property name="label" translatable="yes">Controlle_r:</property> + <property name="use_underline">True</property> + <property name="mnemonic_widget">storage-scsi-index</property> + </object> + <packing> + <property name="left_attach">0</property> + <property name="top_attach">0</property> + </packing> + </child> + <child> + <object class="GtkComboBox" id="storage-scsi-index"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="halign">start</property> + </object> + <packing> + <property name="left_attach">1</property> + <property name="top_attach">0</property> + </packing> + </child> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="left_attach">2</property> + <property name="top_attach">1</property> + </packing> + </child> <child> <object class="GtkComboBox" id="storage-devtype"> <property name="visible">True</property> @@ -205,6 +244,7 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="halign">start</property> + <signal name="changed" handler="on_storage_bustype_changed" swapped="no"/> </object> <packing> <property name="left_attach">1</property> diff --git a/ui/details.ui b/ui/details.ui index 1202db07..5f8cfe74 100644 --- a/ui/details.ui +++ b/ui/details.ui @@ -3559,6 +3559,46 @@ if you know what you are doing.</small></property> <property name="top_attach">0</property> </packing> </child> + <child> + <object class="GtkGrid" id="controller-idx"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="row_spacing">3</property> + <property name="column_spacing">8</property> + <child> + <object class="GtkLabel" id="label67"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="halign">start</property> + <property name="label" translatable="yes">Controlle_r:</property> + <property name="use_underline">True</property> + <property name="mnemonic_widget">controller-index</property> + </object> + <packing> + <property name="left_attach">0</property> + <property name="top_attach">0</property> + </packing> + </child> + <child> + <object class="GtkComboBox" id="controller-index"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="halign">start</property> + <signal name="changed" handler="on_controller_index_combo_changed" swapped="no"/> + </object> + <packing> + <property name="left_attach">1</property> + <property name="top_attach">0</property> + </packing> + </child> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="left_attach">2</property> + <property name="top_attach">0</property> + </packing> + </child> <child> <object class="GtkEntry" id="disk-serial"> <property name="visible">True</property> diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py index d661602d..9205725f 100644 --- a/virtManager/addhardware.py +++ b/virtManager/addhardware.py @@ -97,6 +97,7 @@ class vmmAddHardware(vmmGObjectUI): "on_create_finish_clicked": self._finish, "on_hw_list_changed": self._hw_selected, + "on_storage_bustype_changed": self._change_storage_bustype, "on_storage_devtype_changed": self._change_storage_devtype, "on_mac_address_clicked": self._change_macaddr_use, @@ -196,6 +197,10 @@ class vmmAddHardware(vmmGObjectUI): netmodel_list = self.widget("net-model") self.build_network_model_combo(self.vm, netmodel_list) + # Scsi index combo + self.build_controller_index_combo(self.vm, + self.widget("storage-scsi-index")) + # Disk bus type self.build_disk_bus_combo(self.vm, self.widget("storage-bustype")) @@ -678,6 +683,41 @@ class vmmAddHardware(vmmGObjectUI): model.append([None, _("Hypervisor default")]) combo.set_active(0) + @staticmethod + def build_controller_index_combo(vm, combo): + ignore = vm + model = Gtk.ListStore(str, str) + combo.set_model(model) + uiutil.init_combo_text_column(combo, 1) + model.set_sort_column_id(1, Gtk.SortType.ASCENDING) + combo.set_active(-1) + + @staticmethod + def has_available_scsi_unit(vm, controller_index): + used_unit = 0 + for disk in vm.get_disk_devices(inactive=True): + if disk.bus == "scsi": + if disk.address.controller == controller_index: + used_unit += 1 + if used_unit < 7: + return True + else: + return False + + @staticmethod + def find_available_scsi_unit(vm, controller_index): + max_unit = 7 + avail_unit = 0 + used_list = [] + for disk in vm.get_disk_devices(inactive=True): + if disk.bus == "scsi": + if str(disk.address.controller) == controller_index: + used_list.append(disk.address.unit) + while avail_unit < max_unit: + if avail_unit not in used_list: + return avail_unit + avail_unit += 1 + @staticmethod def build_disk_bus_combo(vm, combo): ignore = vm @@ -718,6 +758,9 @@ class vmmAddHardware(vmmGObjectUI): if vm.conn.is_xen() or vm.conn.is_test(): disk_bus_types.append("xen") + if vm.get_hv_type() in ["qemu", "kvm", "test"]: + disk_bus_types.append("virtio-scsi") + rows = [] for bus in disk_bus_types: rows.append([bus, virtinst.VirtualDisk.pretty_disk_bus(bus)]) @@ -725,10 +768,11 @@ class vmmAddHardware(vmmGObjectUI): model.clear() bus_map = { - "disk": ["ide", "sata", "scsi", "sd", "usb", "virtio", "xen"], + "disk": ["ide", "sata", "scsi", "sd", "usb", "virtio", "xen", + "virtio-scsi"], "floppy": ["fdc"], - "cdrom": ["ide", "sata", "scsi"], - "lun": ["scsi"], + "cdrom": ["ide", "sata", "scsi", "virtio-scsi"], + "lun": ["scsi", "virtio-scsi"], } for row in rows: if row[0] in bus_map[devtype]: @@ -1132,6 +1176,30 @@ class vmmAddHardware(vmmGObjectUI): # Device page listeners # ######################### + def _change_storage_bustype(self, ignore): + bustype = uiutil.get_list_selection( + self.widget("storage-bustype")) + + controller_model = bustype + if controller_model in ["scsi", "virtio-scsi"]: + model = self.widget("storage-scsi-index").get_model() + model.clear() + if controller_model == "scsi": + controller_model = None + for controller in self.vm.get_controller_devices(): + if (controller.type == "scsi" and + controller.model == controller_model): + if not vmmAddHardware.has_available_scsi_unit( + self.vm, controller.index): + continue + model.append([str(controller.index), + str(controller.index)]) + model.append(["new", "New"]) + self.widget("storage-scsi-index").set_active(0) + self.widget("storage-scsi-idx").set_visible(True) + else: + self.widget("storage-scsi-idx").set_visible(False) + def _change_storage_devtype(self, ignore): devtype = uiutil.get_list_selection( self.widget("storage-devtype")) @@ -1429,10 +1497,9 @@ class vmmAddHardware(vmmGObjectUI): self._dev.validate() return ret - def _set_disk_controller(self, disk, controller_model, used_disks): - # Add a SCSI controller with model virtio-scsi if needed + def _set_disk_controller(self, disk, controller_model): disk.vmm_controller = None - if controller_model != "virtio-scsi": + if disk.bus != "scsi": return None # Get SCSI controllers @@ -1440,7 +1507,7 @@ class vmmAddHardware(vmmGObjectUI): ctrls_scsi = [x for x in controllers if (x.type == VirtualController.TYPE_SCSI)] - # Create possible new controller + # Create new controller controller = VirtualController(self.conn.get_backend()) controller.type = "scsi" controller.model = controller_model @@ -1450,23 +1517,7 @@ class vmmAddHardware(vmmGObjectUI): if ctrls_scsi: controller.index = max([x.index for x in ctrls_scsi]) + 1 - # Take only virtio-scsi ones - ctrls_scsi = [x for x in ctrls_scsi - if x.model == controller_model] - - # Save occupied places per controller - occupied = collections.defaultdict(int) - for d in used_disks: - if (d.get_target_prefix() == disk.get_target_prefix() and - d.bus == "scsi"): - num = virtinst.VirtualDisk.target_to_num(d.target) - occupied[num / 7] += 1 - for c in ctrls_scsi: - if occupied[c.index] < 7: - controller = c - break - else: - disk.vmm_controller = controller + disk.vmm_controller = controller return controller.index @@ -1479,9 +1530,12 @@ class vmmAddHardware(vmmGObjectUI): self.widget("storage-cache")) controller_model = None - if (bus == "scsi" and + controller_index = None + scsi_unit = None + if (bus == "virtio-scsi" and self.vm.get_hv_type() in ["qemu", "kvm", "test"] and not self.vm.xmlobj.os.is_pseries()): + bus = "scsi" controller_model = "virtio-scsi" collidelist = [d.path for d in self.vm.get_disk_devices()] @@ -1507,10 +1561,25 @@ class vmmAddHardware(vmmGObjectUI): if d.target not in used: used.append(d.target) - prefer_ctrl = self._set_disk_controller( - disk, controller_model, self.vm.get_disk_devices(inactive=True)) + new_ctrl_idx = self._set_disk_controller(disk, controller_model) + disk.generate_target(used, new_ctrl_idx) + + if bus == "scsi": + controller_index = uiutil.get_list_selection( + self.widget("storage-scsi-index")) + if controller_index != "new": + scsi_unit = vmmAddHardware.find_available_scsi_unit( + self.vm, controller_index) + disk.address.type = "drive" + disk.address.controller = controller_index + disk.address.bus = "0" + disk.address.target = "0" + disk.address.unit = scsi_unit + disk.vmm_controller = None + else: + disk.address.type = "drive" + disk.address.controller = new_ctrl_idx - disk.generate_target(used, prefer_ctrl) except Exception as e: return self.err.val_err(_("Storage parameter error."), e) diff --git a/virtManager/details.py b/virtManager/details.py index a9e0cb79..3c116787 100644 --- a/virtManager/details.py +++ b/virtManager/details.py @@ -30,6 +30,7 @@ import libvirt import virtinst from virtinst import util from virtinst import VirtualRNGDevice +from virtinst import VirtualController from . import vmmenu from . import uiutil @@ -538,7 +539,8 @@ class vmmDetails(vmmGObjectUI): "on_disk_removable_changed": lambda *x: self.enable_apply(x, EDIT_DISK_REMOVABLE), "on_disk_cache_combo_changed": lambda *x: self.enable_apply(x, EDIT_DISK_CACHE), "on_disk_io_combo_changed": lambda *x: self.enable_apply(x, EDIT_DISK_IO), - "on_disk_bus_combo_changed": lambda *x: self.enable_apply(x, EDIT_DISK_BUS), + "on_controller_index_combo_changed": lambda *x: self.enable_apply(x, EDIT_DISK_BUS), + "on_disk_bus_combo_changed": self.disk_bus_combo_changed, "on_disk_format_changed": self.disk_format_changed, "on_disk_serial_changed": lambda *x: self.enable_apply(x, EDIT_DISK_SERIAL), "on_disk_sgio_entry_changed": lambda *x: self.enable_apply(x, EDIT_DISK_SGIO), @@ -1006,6 +1008,10 @@ class vmmDetails(vmmGObjectUI): disk_bus = self.widget("disk-bus") vmmAddHardware.build_disk_bus_combo(self.vm, disk_bus) + # Controller index combo + controller_index = self.widget("controller-index") + vmmAddHardware.build_controller_index_combo(self.vm, controller_index) + # Network model net_model = self.widget("network-model") vmmAddHardware.build_network_model_combo(self.vm, net_model) @@ -1822,6 +1828,41 @@ class vmmDetails(vmmGObjectUI): boot_list.get_selection().emit("changed") self.enable_apply(EDIT_BOOTORDER) + def disk_bus_combo_changed(self, ignore): + controller_model = uiutil.get_list_selection(self.widget("disk-bus")) + + if controller_model in ["scsi", "virtio-scsi"]: + disk = self.get_hw_selection(HW_LIST_COL_DEVICE) + model = self.widget("controller-index").get_model() + model.clear() + current_controller = -1 + i = -1 + if controller_model == "scsi": + controller_model = None + for controller in self.vm.get_controller_devices(): + if (controller.type == "scsi" and + controller.model == controller_model): + i += 1 + if (disk.bus == "scsi" and + disk.address.controller == controller.index): + current_controller = i + if not vmmAddHardware.has_available_scsi_unit( + self.vm, controller.index): + if current_controller != i: + continue + model.append([str(controller.index), + str(controller.index)]) + model.append(["new", "New"]) + if current_controller >= 0: + self.widget("controller-index").set_active(current_controller) + else: + self.widget("controller-index").set_active(0) + self.widget("controller-idx").set_visible(True) + else: + self.widget("controller-idx").set_visible(False) + + self.enable_apply(EDIT_DISK_BUS) + def disk_format_changed(self, ignore): self.widget("disk-format-warn").show() self.enable_apply(EDIT_DISK_FORMAT) @@ -2127,6 +2168,35 @@ class vmmDetails(vmmGObjectUI): if self.edited(EDIT_DISK_BUS): bus = uiutil.get_list_selection(self.widget("disk-bus")) addr = None + controller_model = None + if bus in ["scsi", "virtio-scsi"]: + controller_index = uiutil.get_list_selection( + self.widget("controller-index")) + if bus == "virtio-scsi": + bus = "scsi" + controller_model = "virtio-scsi" + if controller_index != "new": + scsi_bus = "0" + scsi_target = "0" + scsi_unit = vmmAddHardware.find_available_scsi_unit( + self.vm, controller_index) + addr = (controller_index + "/" + scsi_bus + "/" + + scsi_target + "/" + str(scsi_unit)) + else: + controllers = self.vm.get_controller_devices() + ctrls_scsi = [x for x in controllers if (x.type == "scsi")] + + new_ctrl = VirtualController(self.conn.get_backend()) + new_ctrl.type = "scsi" + new_ctrl.model = controller_model + new_ctrl.index = 0 + + if ctrls_scsi: + new_ctrl.index = max([x.index for x in ctrls_scsi]) + 1 + + self.vm.add_device(new_ctrl) + + addr = str(new_ctrl.index) + "/" kwargs["bus"] = bus kwargs["addrstr"] = addr @@ -2681,6 +2751,14 @@ class vmmDetails(vmmGObjectUI): vmmAddHardware.populate_disk_bus_combo(self.vm, devtype, self.widget("disk-bus").get_model()) uiutil.set_list_selection(self.widget("disk-bus"), bus) + if bus == virtinst.VirtualController.TYPE_SCSI: + for controller in self.vm.get_controller_devices(): + if (controller.type == "scsi" and + controller.index == disk.address.controller): + if controller.model == "virtio-scsi": + uiutil.set_list_selection(self.widget("disk-bus"), + "virtio-scsi") + self.widget("disk-serial").set_text(serial or "") button = self.widget("disk-cdrom-connect") diff --git a/virtManager/domain.py b/virtManager/domain.py index a1f59e38..fb658d59 100644 --- a/virtManager/domain.py +++ b/virtManager/domain.py @@ -791,7 +791,12 @@ class vmmDomain(vmmLibvirtObject): editdev.bus = bus if oldbus == bus: - return + if oldbus == "scsi": + controller_index = addrstr.split("/")[0] + if editdev.address.controller == controller_index: + return + else: + return editdev.address.clear() editdev.address.set_addrstr(addrstr) diff --git a/virtinst/device.py b/virtinst/device.py index 4f9dcb2c..a8436177 100644 --- a/virtinst/device.py +++ b/virtinst/device.py @@ -36,7 +36,7 @@ class VirtualDeviceAddress(XMLBuilder): """ Examples: <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> - <address type='drive' controller='0' bus='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> <address type='ccid' controller='0' slot='0'/> <address type='virtio-serial' controller='1' bus='0' port='4'/> """ @@ -68,6 +68,19 @@ class VirtualDeviceAddress(XMLBuilder): self.domain, self.bus = addrstr.split(":", 1) elif addrstr == "spapr-vio": self.type = self.ADDRESS_TYPE_SPAPR_VIO + elif addrstr.count("/") in [1, 2, 3]: + self.type = self.ADDRESS_TYPE_DRIVE + self.controller = addrstr.split("/")[0] + if addrstr.count("/") == 1: + if addrstr.split("/")[1] != "": + self.bus = addrstr.split("/")[1] + elif addrstr.count("/") == 2: + self.bus = addrstr.split("/")[1] + self.target = addrstr.split("/")[2] + elif addrstr.count("/") == 3: + self.bus = addrstr.split("/")[1] + self.target = addrstr.split("/")[2] + self.unit = addrstr.split("/")[3] else: raise ValueError(_("Could not determine or unsupported " "format of '%s'") % addrstr) diff --git a/virtinst/devicedisk.py b/virtinst/devicedisk.py index 73ccb5de..16f860f2 100644 --- a/virtinst/devicedisk.py +++ b/virtinst/devicedisk.py @@ -159,6 +159,8 @@ class VirtualDisk(VirtualDevice): return bus.capitalize() if bus == "virtio": return "VirtIO" + if bus == "virtio-scsi": + return "VirtIO SCSI" return bus @staticmethod -- 2.14.0 _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list