Danek Duvall wrote:
> On Fri, Apr 04, 2008 at 09:07:19PM -0600, Tim Knitter wrote:
> 
>> Well I wasn't able to convince myself :-) and changed it still.
> 
> Oh, no!  :)  I'm afraid I don't understand the new construct, because
> you're no longer ever comparing the BE mountpoint to root.  I think you get
> it right for the "root == '/'" case, but not when root is somewhere else.
> In that case, don't you just end up picking the first inactive BE you run
> across?
> 

It needs one more check after the initial gauntlet.                         

if beVals.get("mountpoint") != root:
         continue

I'll test this out thoroughly once I get a system that isn't fibbing.

> Which I guess leads me to ask a question I hadn't thought to ask before --
> how does a person know what dataset to mount (and where) in order to point
> pkg at it, so that it knows to get all the BE stuff right?

The BE has to be mounted before attempting to do an image update. If a BE isn't 
mounted then an image won't be available anyway, right? So unless we added a 
CLI change for image-update/install/uninstall to handle BE's then the user will 
always be on there own in mounting the BE first. I thought that adding a CLI 
change could come next release if we wanted to go that route.

> 
>> One of my test system was always producing '/' for a -R arg of anything
>> but '/' when instantiating be:
>>
>>                be = bootenv.BootEnv(img.get_root())
>>
>> Not sure why img.get_root() was returning '/' for all cases. I hope it is 
>> just an anomaly.
> 
> I would hope so, too, but anomalies are scary, too.  If you can point me at
> a directory where that's happening for you, please do so.
> 

The preview release test system that I saw this on is not available on SWAN. 
I'm downloading the latest preview image and will reinstall to see if it is 
reproducible. 

If you like you can try it on the latest, untested preview release:
/net/doctor.central/tank/nadkarni_cp_dc/nightly/nightly.0304-1.{iso,usb}

>> http://cr.opensolaris.org/~tsk/863.3/
> 
> client.py:
> 
>   - line 277, et al: Since you're not using "e", there's no need to specify
>     it.

yeah. I realized that after I sent out the WR. err

> 
> bootenv.py:
> 
>   - line 82: You're comparing root against "/ "; I doubt that's what you
>     meant.  Given that, I'll request that you do a full testing of your
>     code before you consider it done.  In fact, I'd strongly recommend
>     adding tests to the testsuite to help you with that.
> 

Yeah I'll manually test the crap out of it. Adding to the test suite can happen 
too, well depending on when the gate closes, it may not happen  before dev 
complete which I now have now idea when that is.
 
>   - line 148: four-space indent on a continuation line.
> 
>   - line 152: list + string == traceback
> 
>   - line 158: " ".join(cmd) is the idiom you want.  And shouldn't that be
>     self.clone_dir instead of self.root?
> 
>     In fact, I'd probably do "cmd += [ self.clone_dir ]" inside the if
>     clause, and then just use cmd by itself in the two places.  Use the
>     other value in the else clause.
> 
>   - line 171: self.self?
> 

all above done.

http://cr.opensolaris.org/~tsk/863.4

Tim

> Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to