xen discuss - Oct 2008 - code review for 6734731 and 6734123

I''d like reviewers for:
    6734731 vif-vnic instances can race against each other
    6734123 xpvd-event logging is dismal

The webrev is at http://dme.org/solaris/webrev/xvm-script-cleanup.

On Tue, Oct 14, 2008 at 02:38:00PM +0100, David Edmondson wrote:
> I''d like reviewers for:
>     6734731 vif-vnic instances can race against each other
>     6734123 xpvd-event logging is dismal
> 
> The webrev is at http://dme.org/solaris/webrev/xvm-script-cleanup.
vif-vnic:83

   echo "${LINK}" >> "${badlinks"}

Typo.

Is warn() actually used (I didn''t check hard) ?

Looks good otherwise, nice cleanup

regards
john

On Wed, Oct 15, 2008 at 03:23:51PM +0100, John Levon
wrote:
> On Tue, Oct 14, 2008 at 02:38:00PM +0100, David Edmondson wrote:
> 
> > I''d like reviewers for:
> >     6734731 vif-vnic instances can race against each other
> >     6734123 xpvd-event logging is dismal
> > 
> > The webrev is at http://dme.org/solaris/webrev/xvm-script-cleanup.
> 
> vif-vnic:83
> 
>    echo "${LINK}" >> "${badlinks"}
> 
> Typo.
Good catch, thanks.
> Is warn() actually used (I didn''t check hard) ?
In the case where the lock is forced.
> Looks good otherwise, nice cleanup
Thanks. I also noticed that the Makefile was out of date, so I''ll fix
these and update the webrev.

> I'd like reviewers for:
> 6734731 vif-vnic instances can race against each other
> 6734123 xpvd-event logging is dismal
src/scripts/xpvd-misc lines 160, 164:
  ${file} should be quoted, in case it contains spaces

    if su - xvm -c "test ${op} \"${file}\""; then

    owner=‘ls -lL "${file}" | awk ’{ print $3; }’‘



src/scripts/xpvd-misc, line 55
  should use a shell integer compare using -eq,
  not a string compare

    if [ "${loop}" -eq "${timeout}" ]; then
--
This message posted from opensolaris.org
_______________________________________________
xen-discuss mailing list
xen-discuss@opensolaris.org

On Wed, Oct 15, 2008 at 09:11:28AM -0700, J??rgen Keil
wrote:
> > I''d like reviewers for:
> > 6734731 vif-vnic instances can race against each other
> > 6734123 xpvd-event logging is dismal
> 
> src/scripts/xpvd-misc lines 160, 164:
>   ${file} should be quoted, in case it contains spaces
> 
>     if su - xvm -c "test ${op} \"${file}\""; then
> 
>     owner=???ls -lL "${file}" | awk ???{ print $3; }??????
> 
> 
> 
> src/scripts/xpvd-misc, line 55
>   should use a shell integer compare using -eq,
>   not a string compare
> 
>     if [ "${loop}" -eq "${timeout}" ]; then
Thanks. I''ll apply these updates and republish.

The webrev is updated based on the feedback. There''s also a little
more cleanup (hardly anything) and an update to .hgignore for
SUNWvdisk''s packaging bits.

http://dme.org/solaris/webrev/xvm-script-cleanup/sunos.hg/

On Fri, Oct 17, 2008 at 04:03:19PM +0100, David Edmondson wrote:
> The webrev is updated based on the feedback. There''s also a little
> more cleanup (hardly anything) and an update to .hgignore for
> SUNWvdisk''s packaging bits.
> 
> http://dme.org/solaris/webrev/xvm-script-cleanup/sunos.hg/
Looks good

regards
john