Re: [PATCH master 2/2] Add KVM chroot feature

[Balazs Lecz]

On Thu, May 27, 2010 at 23:55, Michael Hanselmann <han...@google.com> wrote:

> 2010/5/26 Balazs Lecz <le...@google.com>:
> > +  def _InstanceChrootQuarantineDir(cls, instance_name):
> > +    """Returns the name of the KVM chroot quarantine dir of the instance
> > +
> > +    """
> > +    return utils.PathJoin(cls._CHROOT_QUARANTINE_DIR, "%s_%s"
> > +                          % (instance_name,
> utils.TimestampForFilename()))
>
> The operator goes to the previous line.


Fixed.


> Why don't you use
> tempfile.mkdtemp? Is there any code avoiding collisions?
>
> tempfile.mkdtemp(dir=cls._CHROOT_QUARANTINE_DIR,
>                 prefix="%s-%s-" % (instance_name, utils.Time…))
>
> Michael
>

You are right, the timestamp itself doesn't provide collision avoidance
guarantee.
I've add the mkdtemp call as suggested.

/leczb