On Wed, 2010-03-24 at 16:58 -0700, Karen Tung wrote:
> On 03/24/10 03:29, Alexander Eremin wrote:
> >> On 03/15/10 06:55 AM, Alexander Eremin wrote:
> >>
> >>>> On 03/ 5/10 08:11 AM, Dave Miner wrote:
> >>>>
> >>>>
> >>>>> On 03/ 4/10 12:43 PM, Alexander Eremin wrote:
> >>>>>
> >>>>>
> >>>>>>> On 03/ 3/10 09:15 AM, Alexander Eremin wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>> Hi all,
> >>>>>>>> Please review the short fixes for:
> >>>>>>>> 13276 - live-fs-root needs to be more careful
> >>>>>>>>
> >>>>>>>>
> >>>> about
> >>>>
> >>>>
> >>>>>>> what USB device
> >>>>>>>
> >>>>>>>
> >>>>>>>> Webrev's available at
> >>>>>>>>
> >>>>>>>>
> >>>>>>> http://cr.opensolaris.org/~alhazred/13276/
> >>>>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>> In media-fs-root, wouldn't it make more sense
> >>>>>>>
> >> to
> >>
> >>>>>>>
> >>>>>>>
> >>>> just
> >>>>
> >>>>
> >>>>>>> unmount and
> >>>>>>> continue (rather than exiting fatally),
> >>>>>>>
> >> assuming
> >>
> >>>>>>>
> >>>>>>>
> >>>> that
> >>>>
> >>>>
> >>>>>>> we will find a USB
> >>>>>>> device with the right version?
> >>>>>>>
> >>>>>>> Dave
> >>>>>>> _______________________________________________
> >>>>>>> caiman-discuss mailing list
> >>>>>>> caiman-discuss at opensolaris.org
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>
> >> http://mail.opensolaris.org/mailman/listinfo/caiman-di
> >>
> >>>>
> >>>>
> >>>>>>> scuss
> >>>>>>>
> >>>>>>>
> >>>>>> I've updated the fix to reflect the feedback
> >>>>>>
> >> from
> >>
> >>>>>>
> >>>>>>
> >>>> Dave.
> >>>>
> >>>>
> >>>>>> http://cr.opensolaris.org/~alhazred/13276/
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>> Sorry I didn't spend more time thinking about
> >>>>>
> >> this
> >>
> >>>>>
> >>>>>
> >>>> initially, but
> >>>>
> >>>>
> >>>>> there is one other thing about the fix that is
> >>>>>
> >>>>>
> >>>> somewhat a problem.
> >>>>
> >>>>
> >>>>> Hard-coding usage of the build number from
> >>>>>
> >>>>>
> >>>> /etc/release in the CD
> >>>>
> >>>>
> >>>>> volume ID is OK for development builds, but
> >>>>>
> >> likely
> >>
> >>>>>
> >>>>>
> >>>> not what we want
> >>>>
> >>>>
> >>>>> for release builds. The reason is that this will
> >>>>>
> >>>>>
> >>>> cause the CD to be
> >>>>
> >>>>
> >>>>> displayed on the desktop as "OpenSolaris_134a"
> >>>>>
> >>>>>
> >>>> (using the expected
> >>>>
> >>>>
> >>>>> build for 2010.03 as the example), when we would
> >>>>>
> >>>>>
> >>>> prefer, from a
> >>>>
> >>>>
> >>>>> marketing/documentation point of view to have it
> >>>>>
> >>>>>
> >>>> display the release
> >>>>
> >>>>
> >>>>> name and not a build number; in other words,
> >>>>>
> >> either
> >>
> >>>>>
> >>>>>
> >>>> "OpenSolaris
> >>>>
> >>>>
> >>>>> 2010.03" or maybe "OpenSolaris_2010.03" would be
> >>>>>
> >>>>>
> >>>> preferred for release
> >>>>
> >>>>
> >>>>> builds. Thoughts on how to modify to allow that?
> >>>>>
> >>>>> Dave
> >>>>>
> >>>>>
> >>>>>
> >>>> We can get more the marketing-friendly name for
> >>>>
> >> the
> >>
> >>>> .volumeid file from
> >>>> the grub title
> >>>> entry in the DC manifests. For development
> >>>>
> >> builds,
> >>
> >>>> the grub
> >>>> title entry will be the value from /etc/release.
> >>>>
> >> For
> >>
> >>> release builds, RE
> >>>
> >>>> defines the
> >>>> grub title entry in the manifest, the
> >>>>
> >> setup_grub.py
> >>
> >>>> finalizer script
> >>>> uses that
> >>>> value for the grub menu title instead of values
> >>>>
> >> from
> >>
> >>>> /etc/release.
> >>>>
> >>>> For the .volumeid file, if the special title is
> >>>> defined, that will be
> >>>> used. Otherwise,
> >>>> the string from /etc/release can be used.
> >>>>
> >>>> Thanks,
> >>>>
> >>>> --Karen
> >>>>
> >>>> _______________________________________________
> >>>> caiman-discuss mailing list
> >>>> caiman-discuss at opensolaris.org
> >>>>
> >>>>
> >> http://mail.opensolaris.org/mailman/listinfo/caiman-di
> >>
> >>>> scuss
> >>>>
> >>>>
> >>> Karen, Dave,
> >>> could I please ask you to take a look at updated
> >>>
> >> fix?
> >>
> >>> Thanks,
> >>> Alex
> >>>
> >>>
> >> Hi Alex,
> >>
> >> Here are my comments:
> >>
> >> slimcd_gen_cd_content:
> >> - I don't think this is the right place to copy the
> >> .volumeid from the
> >> root archive
> >> to the root of the CD. First of all, the purpose of
> >> this finalizer
> >> script is to generate
> >> the list of files that should be copied from the
> >> LiveCD to the installed
> >> system, and
> >> copying the .volumneid file is not related to that.
> >> Most importantly,
> >> he slimcd_gen_cd_content
> >> script is only executed for Live CDs, it will not be
> >> executed for other
> >> image types, such
> >> as AI images and text installer images. As a result,
> >> bootable AI USB
> >> images, and text
> >> installer USB images will still suffer the bug. IMO,
> >> the copying of the
> >> .volumneid file
> >> should be included in
> >> "post_boot_archive_pkg_image_mod" finalizer
> >> script, which
> >> is a script all image types will use.
> >>
> >> create_iso, lines 88-94:
> >> - Since we already computed the string and it is
> >> stored in
> >> $PKG_IMG_PATH/.volumeid, I think we can just retrieve
> >> the string from
> >> the file
> >> to use here instead of computing it again. In the
> >> future, if we ever decide
> >> to compute the string differently, the changes only
> >> needs to be done in
> >> 1 place
> >> instead of multiple places.
> >>
> >> create_iso, line 105 and line 121.
> >> - After we get the value for the $RELEASE string, I
> >> think it is better
> >> to define
> >> a variable that will be used for the -V option,
> >> instead of hard coding
> >> "$DISTRO_NAME $RELEASE"
> >>
> >> Thanks,
> >>
> >> --Karen
> >>
> >>
> >> _______________________________________________
> >> caiman-discuss mailing list
> >> caiman-discuss at opensolaris.org
> >> http://mail.opensolaris.org/mailman/listinfo/caiman-di
> >> scuss
> >>
> >>
> > Hi Karen,
> > webrev is updated accordingly to your feedback
> > and tested for sparc AI and x86 LiveCD.
> > http://cr.opensolaris.org/~alhazred/13276/
> >
> > Thanks,
> > Alex
> >
> Hi Alex,
>
> post_boot_archive_pkg_image_mod, line 62. That comment says the
> BA_BUILD value is not used, but it is used after this change.
> Please update the comment.
Yes
> Everything else looks good to me now.
>
> Thanks,
>
> --Karen
Karen, thank you very much for reviewing!
--
Alex
