[virt-tools-list] [virt-manager PATCH] details: add UI interface for LXC user namespace

[Cole Robinson]

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 at cn.fujitsu.com>
> ---
>  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