On 09/18/2013 09:29 AM, Giuseppe Scrivano wrote: > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1001773 > > Signed-off-by: Giuseppe Scrivano <gscrivan@xxxxxxxxxx> > --- > ui/vmm-add-hardware.ui | 235 +++++++++++++++++++++++++++++++++++++++ > ui/vmm-details.ui | 271 +++++++++++++++++++++++++++++++++++++++++++++ > virtManager/addhardware.py | 107 ++++++++++++++++++ > virtManager/details.py | 41 ++++++- > virtManager/domain.py | 3 + > virtManager/uihelpers.py | 58 ++++++++++ > 6 files changed, 713 insertions(+), 2 deletions(-) > Please split this patch into two parts: one patch for the details page handling, and one patch for the addhardware handling. A few UI comments here: - On the details page, use a frame like the other UI pages with a bold title. It's kind of pointless but lets not deviate unnecessarily. - Switch the details page 'table' to a 'grid'. The only way I found to do this was to edit the ui file and change the GtkTable to GtkGrid, and then adjust the spacing in glade (f19 glade doesn't seem to have a grid option). No other changes should be needed for it to just work. - Hide the RNG rows in the details page that aren't valid for the rng type. Once you've converted to grid, you can use the following helper like uihelpers.set_grid_row_visible(self.widget("rng-device"), not is_egd) That will hide every element that's in the same row as rng-device, and get the spacing correct. - In the details page, drop the 'ypad' for each label, the only row spacing should be set at the table/grid level and it should be '4' (which it currently is). I realize this was just following the TPM example as well, but it needs to be fixed as described above. - In addhardware, you use a notebook to separate the different UI for type=device vs type=egd. I'd recommend just using a single grid to hold all the elements, and doing the set_grid_row_visible trick. We use this on other pages, and it has the advantage that you don't need to duplicate UI fields across the notebook pages if there are shared properties (doesn't apply for rng but still). It also gives proper alignment for the Type: field WRT the rest of the UI elements. Some code comments below: > diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py > index e13482c..0fe6d06 100644 > --- a/virtManager/addhardware.py > +++ b/virtManager/addhardware.py > @@ -54,6 +54,7 @@ PAGE_FILESYSTEM = 10 > PAGE_SMARTCARD = 11 > PAGE_USBREDIR = 12 > PAGE_TPM = 13 > +PAGE_RNG = 14 > > char_widget_mappings = { > "source_path" : "char-path", > @@ -105,6 +106,8 @@ class vmmAddHardware(vmmGObjectUI): > "on_fs_source_browse_clicked": self.browse_fs_source, > > "on_usbredir_type_changed": self.change_usbredir_type, > + > + "on_rng_type_changed": self.change_rng_type, > }) > self.bind_escape_key_close() > > @@ -310,6 +313,14 @@ class vmmAddHardware(vmmGObjectUI): > combo = self.widget("tpm-type") > uihelpers.build_tpm_type_combo(self.vm, combo) > > + # RNG widgets > + combo = self.widget("rng-type") > + uihelpers.build_rng_type_combo(self.vm, combo) > + combo = self.widget("rng-backend-type") > + uihelpers.build_rng_backend_type_combo(self.vm, combo) > + combo = self.widget("rng-backend-mode") > + uihelpers.build_rng_backend_mode_combo(self.vm, combo) > + I usually only stick these bits in uihelpers if they are actually used in two places (usually addhw and details). I don't think there's much use in allowing to editing an RNG device in details, so this stuff likely never needs to be shared. Just track it in addhardware.py > # Available HW options > is_local = not self.conn.is_remote() > is_storage_capable = self.conn.is_storage_capable() > @@ -376,6 +387,7 @@ class vmmAddHardware(vmmGObjectUI): > True, None) > add_hw_option("TPM", "device_cpu", PAGE_TPM, > True, None) > + add_hw_option("RNG", "system-run", PAGE_RNG, True, None) > > def reset_state(self): > # Storage init > @@ -474,6 +486,11 @@ class vmmAddHardware(vmmGObjectUI): > widget = notebook.get_nth_page(page) > widget.hide() > > + # RNG params > + self.widget("rng-device").set_text("/dev/random") > + self.widget("rng-host").set_text("localhost") > + self.widget("rng-service").set_text("708") > + > self.set_hw_selection(0) > > ######################### > @@ -771,6 +788,44 @@ class vmmAddHardware(vmmGObjectUI): > typestr = typ.get_model().get_value(typ.get_active_iter(), 0) > return typestr > > + # RNG getters > + def get_config_rng_type(self): > + src = self.widget("rng-type") > + idx = src.get_active() > + if idx < 0: > + return None > + > + selected_type = src.get_model()[idx][0] > + return selected_type > + > + def get_config_rng_device(self): > + if self.get_config_rng_type() == virtinst.VirtualRNGDevice.TYPE_RANDOM: > + return self.widget("rng-device").get_text() > + > + return None > + > + def get_config_rng_host(self): > + if self.get_config_rng_type() == virtinst.VirtualRNGDevice.TYPE_EGD: > + return self.widget("rng-host").get_text() > + > + return None > + > + def get_config_rng_service(self): > + if self.get_config_rng_type() == virtinst.VirtualRNGDevice.TYPE_EGD: > + return self.widget("rng-service").get_text() > + > + return None > + > + def get_config_rng_backend_type(self): > + active = self.widget("rng-backend-type").get_active() > + model = self.widget("rng-backend-type").get_model() > + return model[active][0] > + > + def get_config_rng_backend_mode(self): > + active = self.widget("rng-backend-mode").get_active() > + model = self.widget("rng-backend-mode").get_model() > + return model[active][0] > + > ################ > # UI listeners # > ################ > @@ -929,6 +984,8 @@ class vmmAddHardware(vmmGObjectUI): > return _("USB Redirection") > if page == PAGE_TPM: > return _("TPM") > + if page == PAGE_RNG: > + return _("Random Number Generator") > > if page == PAGE_CHAR: > char_class = self.get_char_type() > @@ -990,6 +1047,14 @@ class vmmAddHardware(vmmGObjectUI): > uihelpers.set_grid_row_visible(self.widget("usbredir-host-box"), > showhost) > > + def change_rng_type(self, ignore1): > + model = self.get_config_rng_type() > + if model is None: > + return > + > + page_idx = virtinst.VirtualRNGDevice.TYPES.index(model) > + self.widget("rng-host-notebook").set_current_page(page_idx) > + > # FS listeners > def browse_fs_source(self, ignore1): > self._browse_file(self.widget("fs-source"), isdir=True) > @@ -1166,6 +1231,8 @@ class vmmAddHardware(vmmGObjectUI): > return self.validate_page_usbredir() > elif page_num == PAGE_TPM: > return self.validate_page_tpm() > + elif page_num == PAGE_RNG: > + return self.validate_page_rng() > > def validate_page_storage(self): > bus, device = self.get_config_disk_target() > @@ -1539,6 +1606,46 @@ class vmmAddHardware(vmmGObjectUI): > except Exception, e: > return self.err.val_err(_("TPM device parameter error"), e) > > + def validate_page_rng(self): > + conn = self.conn.get_backend() > + model = self.get_config_rng_type() > + > + if model == virtinst.VirtualRNGDevice.TYPE_RANDOM: > + if not self.get_config_rng_device(): > + return self.err.val_err(_("RNG selection error."), > + _("A device must be specified.")) > + elif model == virtinst.VirtualRNGDevice.TYPE_EGD: > + if not self.get_config_rng_host(): > + return self.err.val_err(_("RNG selection error."), > + _("The EGD host must be specified.")) > + > + if not self.get_config_rng_service(): > + return self.err.val_err(_("RNG selection error."), > + _("The EGD service must be specified.")) > + else: > + return self.err.val_err(_("RNG selection error."), > + _("Invalid RNG type.")) > + > + value_mappings = { > + "backend_mode" : "connect", > + "backend_type" : self.get_config_rng_backend_type(), > + "backend_source_mode" : self.get_config_rng_backend_mode(), > + "backend_source_host" : self.get_config_rng_host(), > + "backend_source_service" : self.get_config_rng_service(), > + "device" : self.get_config_rng_device(), > + "model" : "virtio" > + } > + I'd drop the explicit model=virtio and just let the HV fill it in. Hardcoding things like sometimes comes back to haunt us. > + try: > + self._dev = virtinst.VirtualRNGDevice(conn) > + self._dev.type = self.get_config_rng_type() > + for param_name, val in value_mappings.items(): > + if self._dev.supports_property(param_name): > + setattr(self._dev, param_name, val) > + except Exception, e: > + return self.err.val_err(_("TPM device parameter error"), e) > + > + > #################### > # Unsorted helpers # > #################### > diff --git a/virtManager/details.py b/virtManager/details.py > index a02778c..d08ef71 100644 > --- a/virtManager/details.py > +++ b/virtManager/details.py > @@ -122,14 +122,16 @@ EDIT_TPM_TYPE, > HW_LIST_TYPE_FILESYSTEM, > HW_LIST_TYPE_SMARTCARD, > HW_LIST_TYPE_REDIRDEV, > - HW_LIST_TYPE_TPM) = range(19) > + HW_LIST_TYPE_TPM, > + HW_LIST_TYPE_RNG) = range(20) > > remove_pages = [HW_LIST_TYPE_NIC, HW_LIST_TYPE_INPUT, > HW_LIST_TYPE_GRAPHICS, HW_LIST_TYPE_SOUND, HW_LIST_TYPE_CHAR, > HW_LIST_TYPE_HOSTDEV, HW_LIST_TYPE_DISK, HW_LIST_TYPE_VIDEO, > HW_LIST_TYPE_WATCHDOG, HW_LIST_TYPE_CONTROLLER, > HW_LIST_TYPE_FILESYSTEM, HW_LIST_TYPE_SMARTCARD, > - HW_LIST_TYPE_REDIRDEV, HW_LIST_TYPE_TPM] > + HW_LIST_TYPE_REDIRDEV, HW_LIST_TYPE_TPM, > + HW_LIST_TYPE_RNG] > > # Boot device columns > (BOOT_DEV_TYPE, > @@ -1256,6 +1258,8 @@ class vmmDetails(vmmGObjectUI): > self.refresh_redir_page() > elif pagetype == HW_LIST_TYPE_TPM: > self.refresh_tpm_page() > + elif pagetype == HW_LIST_TYPE_RNG: > + self.refresh_rng_page() > else: > pagetype = -1 > except Exception, e: > @@ -3143,6 +3147,34 @@ class vmmDetails(vmmGObjectUI): > # Device type specific properties, only show if apply to the cur dev > show_ui("device_path") > > + def refresh_rng_page(self): > + dev = self.get_hw_selection(HW_LIST_COL_DEVICE) > + values = {"rng-type" : "type", > + "rng-device" : "device", > + "rng-host" : "backend_source_host", > + "rng-service" : "backend_source_service", > + "rng-mode" : "backend_source_mode", > + "rng-backend-type" : "backend_type", > + "rng-rate-bytes" : "rate_bytes", > + "rng-rate-period" : "rate_period"} > + rewriter = {"rng-type" : lambda x: > + virtinst.VirtualRNGDevice.get_pretty_type(x), > + "rng-backend-type" : lambda x: > + virtinst.VirtualRNGDevice.get_pretty_backend_type(x), > + "rng-mode" : lambda x: > + virtinst.VirtualRNGDevice.get_pretty_mode(x) > + } I prefer spacing like rewriter = { foo: bar, } though you'll probably find examples in the code of your format :) > + > + for k in values: You can do 'for k, prop in values.items()' Thanks, Cole _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
