> -----Original Message----- > From: Cole Robinson [mailto:crobinso@xxxxxxxxxx] > Sent: Thursday, February 20, 2014 12:22 AM > To: Chen Hanxiao; virt-tools-list@xxxxxxxxxx > Subject: Re: [virt-manager PATCH] details: add UI interface for LXC > user namespace > > On 02/19/2014 06:34 AM, Chen Hanxiao wrote: > > We could config user namespace for LXC container > > in details->overview page. > > > > Signed-off-by: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx> > > --- > > ui/details.ui | 239 > +++++++++++++++++++++++++++++++++++++++++++++++-- > > virtManager/details.py | 58 +++++++++++- > > virtManager/domain.py | 15 ++++ > > 3 files changed, 305 insertions(+), 7 deletions(-) > > > > Some UI hints to make things more consistent: > > - Expander label should be bold, and rename it to 'User Namespace' > - Rename 'Config User Namespace' to 'Enable user namespace' > - The box/grid under the expander should be inside a GtkAlignment, and > indented. Copy the indentation of the 'Basic Details' section > - The fields should be spin boxes. > - The grid should use column and row spacing of 6, which is the standard we > use for most other grids. > - Capitalize the column names (target, count) > - 'Config User Namespace' needs to enable the 'apply' button when its changed. > - If the VM has an <idmap> block, and I uncheck 'Config User Namespace', it > should do idmap.clear() and remove the entire block from the XML. > > > diff --git a/virtManager/details.py b/virtManager/details.py > > index 72e79da..f4b1339 100644 > > --- a/virtManager/details.py > > +++ b/virtManager/details.py > > @@ -103,7 +103,9 @@ EDIT_FS, > > > > EDIT_HOSTDEV_ROMBAR, > > > > -) = range(1, 42) > > +EDIT_IDMAP, > > + > > +) = range(1, 43) > > > > It's a minor point, but move this EDIT_IDMAP definition up near EDIT_NAME and > EDIT_DESC, we group the values by what page they appear on. > > > > > # Columns in hw list model > > @@ -579,6 +581,13 @@ class vmmDetails(vmmGObjectUI): > > "on_overview_name_changed": lambda *x: > self.enable_apply(x, EDIT_NAME), > > "on_overview_title_changed": lambda *x: self.enable_apply(x, > EDIT_TITLE), > > "on_machine_type_changed": lambda *x: self.enable_apply(x, > EDIT_MACHTYPE), > > + "on_idmap_uid_start_changed": lambda *x: > self.enable_apply(x, EDIT_IDMAP), > > + "on_idmap_uid_target_changed": lambda *x: > self.enable_apply(x, EDIT_IDMAP), > > + "on_idmap_uid_count_changed": lambda *x: > self.enable_apply(x, EDIT_IDMAP), > > + "on_idmap_gid_start_changed": lambda *x: > self.enable_apply(x, EDIT_IDMAP), > > + "on_idmap_gid_target_changed": lambda *x: > self.enable_apply(x, EDIT_IDMAP), > > + "on_idmap_gid_count_changed": lambda *x: > self.enable_apply(x, EDIT_IDMAP), > > + "on_config_idmap_check_toggled": self.toggle_idmap_check, > > > > "on_config_vcpus_changed": self.config_vcpus_changed, > > "on_config_maxvcpus_changed": > self.config_maxvcpus_changed, > > @@ -1600,6 +1609,24 @@ class vmmDetails(vmmGObjectUI): > > if edittype not in self.active_edits: > > self.active_edits.append(edittype) > > > > + def toggle_idmap_check(self, src): > > + Name = ["uid-start", "uid-target", "uid-count", > > + "gid-start", "gid-target", "gid-count"] > > + for name in Name: > > + self.widget(name).set_sensitive(src.get_active()) > > + IdMap = self.vm.get_idmap() > > + if IdMap.uid_start is not None: > > + for name in Name: > > + IdMap_proper = getattr(IdMap, name.replace("-", "_")) > > + self.widget(name).set_text(str(IdMap_proper)) > > + elif src.get_active(): > > + self.widget("uid-start").set_text('0') > > + self.widget("uid-target").set_text('1000') > > + self.widget("uid-count").set_text('10') > > + self.widget("gid-start").set_text('0') > > + self.widget("gid-target").set_text('1000') > > + self.widget("gid-count").set_text('10') > > + > > Hmm, I don't like that this is duplicating behavior in the refresh_overview > function. > > I recommend changing the check button to hide the entire idmap grid. Then you > don't need to worry about setting values here, just set the default values in > refresh_overview if the guest doesn't have any idmap block. > > FYI I'm offline until next week, so I'll do follow up reviews after Monday. > > - Cole Will do in v2 patch, thanks for your comments. PS: have a nice weekend. -Chen _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
