On 10/27/10 09:21, Keith Mitchell wrote:
zfs.py
line 83-ish I'm not sure if I've missed it somewhere but I didn't
anything that validates the "short_name" to make sure it's a valid
snapshot name. For example if it's already in a snapshot name format
(already has a @ in it) will this be checked? If not then the call
to zfs snapshot at line 80 will fail.
The calls to zfs.snapname from the engine base the short_name
variable on checkpoint names. I'm not certain if checkpoint names
have an "illegal characters" check anywhere (I don't remember seeing
one in the engine review), so I think it's entirely possible to have
an "@" character in the checkpoint name / short_name.
Not real sure what the right fix should be. In theory, users won't
name something with a leading "@" character, but after working in
support for a couple of years, I can guarantee they will. Just to
break it and file a bug.
Karen/Keith? What's your take on this? I could do a
short_name.lstrip("@") in the snapname method of zfs.py to guarantee
there's no leading "@" character, but there could be others scattered
in the checkpoint name. I think the proper approach here is to trap
in the register_checkpoints() method in the engine with a list of
invalid characters for checkpoints / snapshots (/, @, #, !, etc.).
That will prevent zfs.snapshot() from bombing with an invalid snap name.
Sorry, an InsufficientCaffeineError occurred and I didn't correlate
our IRC chat with the email response I sent to Evan. In my first
response to Evan, I mentioned that the snapname function is fine as
written - it's just a helper to turn short names into long names.
Since the Dataset class is just a utility, it shouldn't try to be
*too* helpful, and I think the behavior should be unchanged.
I think the engine itself should either verify that checkpoints don't
have invalid characters (during register_checkpoint), *or* just strip
out the invalid characters before calling zfs.snapshot. I believe only
ASCII letters, numbers, dots, dashes and underscores are allowed
(there's no harm in the engine being *more* restrictive than ZFS)
- Keith
I think it would be more confusing for the engine to strip out invalid
ZFS characters before calling zfs.snapshot().
I do not see any harm in restricting checkpoint names to have only ASCII
letters, numbers, dots, dashes and underscore,
and less than 256 characters.
I will add this check in register_checkpoint, and throw an error for any
checkpoint name that violates
the rules.
Thanks,
--Karen
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
