[libvirt] [sandbox PATCH 4/6] Add support for --homedir and --username setting for virt-sandbox-service

[Daniel P. Berrange]

On Thu, Apr 18, 2013 at 09:56:59AM -0400, dwalsh at redhat.com wrote:
> From: Dan Walsh <dwalsh at redhat.com>
> 
> Also default --homedir, --username, --gid all off of the --uid settings.
> But allow the admin to override if required.
> ---
>  bin/virt-sandbox-service | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
> index 308b871..d869e5d 100755
> --- a/bin/virt-sandbox-service
> +++ b/bin/virt-sandbox-service
> @@ -28,6 +28,7 @@ import exceptions
>  import rpm
>  from subprocess import Popen, PIPE, STDOUT
>  import gettext
> +import pwd
>  
>  if os.path.exists("/sys/fs/selinux"):
>      import selinux
> @@ -83,7 +84,7 @@ class Container:
>          self.file_type = file_type
>  
>      def set_uid(self, uid):
> -        return self.config.set_userid(uid)
> +        self.config.set_userid(uid)
>  
>      def get_uid(self):
>          return self.config.get_userid(uid)
> @@ -94,6 +95,18 @@ class Container:
>      def get_gid(self):
>          return self.config.get_groupid(gid)
>  
> +    def set_username(self, username):
> +        self.config.set_username(username)
> +
> +    def get_username(self):
> +        return self.config.get_username()
> +
> +    def set_homedir(self, homedir):
> +        self.config.set_homedir(homedir)
> +
> +    def get_homedir(self):
> +        return self.config.get_homedir()
> +
>      def get_config_path(self, name = None):
>          if not name:
>              name = self.name
> @@ -853,6 +866,14 @@ def create(args):
>      if args.security:
>          container.set_security(args.security)
>      container.set_uid(args.uid)
> +    if not args.homedir:
> +        args.homedir = pwd.getpwuid(args.uid).pw_dir
> +    container.set_homedir(args.homedir)
> +    if not args.username:
> +        args.username = pwd.getpwuid(args.uid).pw_name
> +    container.set_username(args.username)
> +    if not args.gid:
> +        args.gid = args.uid

This is wrong - you can't assume the gid + uid match. You
need to lookup the default group for the uid in question.

> @@ -1051,9 +1072,11 @@ def gen_create_args(subparser):
>      parser.add_argument("-f", "--filetype", dest="file_type",
>                          default=c.get_file_type(),
>                          help=_("SELinux file type to assign to content within the sandbox.  Default: %s") % c.get_file_type())
> +    parser.add_argument("--homedir", dest="homedir",
> +                        help=_("Specify the homedir for the container. Default: UID homedir."))
>      parser.add_argument("-G", "--gid", dest="gid",
> -                        default=0, type=int,
> -                        help=_("Specify the gid for the container"))
> +                        default=None, type=int,
> +                        help=_("Specify the gid for the container. Default: UID."))

That should say  "Default: the primary GID for the UID"

>      parser.add_argument("-i", "--imagesize", dest="imagesize", default = None,
>                         action=SizeAction,
>                         help=_("create image of this many megabytes."))
> @@ -1069,9 +1092,11 @@ def gen_create_args(subparser):
>                          action=CheckUnit,
>                          dest="unitfiles", default=[],
>                          help=_("Systemd Unit file to run within the Service sandbox container. Commands cannot be specified with unit files."))
> +    parser.add_argument("--username", dest="username",
> +                        help=_("Specify the username for the container. Default: UID username."))
>      parser.add_argument("-U", "--uid", dest="uid",
>                          default=0,type=int,
> -                        help=_("Specify the uid for the container"))
> +                        help=_("Specify the uid for the container. Default: 0."))

Actually the default is the UID of the person invoking the command.
We shouldn't assume that is root.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|