On 11/10/10 04:20 PM, Danek Duvall wrote:
Shawn Walker wrote:
On 11/ 6/10 06:23 PM, Danek Duvall wrote:
Shawn Walker wrote:
http://cr.opensolaris.org/~swalker/pkg-iformat/
image.py:
- line 487: Can you explain why this is safe?
Because configuration classes provide a set of declarations that
define what each version of the image configuration looks like, and
enforce that view of the configuration on load.
I'm not sure how you could enforce a version 3 reading of a version 7
configuration. If you try to load an image whose config file is version 7,
will creating the ImageConfig object fail?
The version here effectively specifies the initial version of the
configuration to use if one can't be determined from the configuration
file itself. In short, if we specify 3 here, but on load the config
class discovers it's 7, it will auto-correct.
- line 632: Can you explain why this is safe?
Based on the structure, I have no reason to believe otherwise.
I guess this is the same as the previous question, then -- what happens if
version 7 has a cache directory, and so you assume that it's version 3
instead?
This is a fallback path that only happens when we couldn't determine the
version of the image from the configuration file. In the case that you
have a version 7 image that looks like a version 3 image, and we have no
configuration file to tell us otherwise, it seems sane to make
assumptions based on structure in the absence of all other identifying
information.
- line 638: this can move out one level
I need it to reload in both cases. Otherwise, I'm not understanding.
I was thinking this could move after line 652, but then it would happen for
the "if purge" case, which isn't right; I'm not sure if I was thinking
something else, or if I just missed that.
As I mentioned above, this whole block is a fallback path for when we
couldn't determine the image version ahead of time. So by forcing a
reload of image configuration, it ensures the configuration definitions
match expectations in the event that the configuration didn't contain
version information of its own.
...
- line 1036: should we handle EXDEV specially?
I wasn't aware we were going to support random splits of /var/pkg into
separate filesystems. And in particular, it seems odd that /var/pkg
would be a separate filesystem.
I could see /var/pkg as a separate filesystem, though splitting it further
would seem odd to me. I don't mind leaving it as is, until there's demand.
Making /var/pkg a mountpoint would make it very difficult to make this
an atomic operation since I couldn't simply rename the entire image
metadata directory into place anymore :(
- line 3038: Should we warn the user that what they're doing is not
necessarily representative of the command that would actually get run?
I don't know. If so, what do you want the message to be?
Something along the lines of "A newer version of pkg(5) might be required,
but insufficient permissions prevented pkg(5) from discovering this; any
subsequent output may not be representative of the command that would
actually perform the requested operation."
Dunno if it's worth it.
The only thing I don't like about that message is that it's misleading
in the case that you're updating a user image or an image not rooted at
'/' that you don't have permission to modify.
Do you still want this message?
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
