Menu

#308 "fru edit" no longer works for non-zero fields

version-1.8.14
closed-fixed
None
5
2014-04-25
2014-04-18
No

The "fru edit" command has a couple of regressions since v1.8.13.

Using v1.8.13:
$ ipmitool fru edit 0 field c 1 testtest
Updating Field 'testtest' with 'testtest' ...

Using v1.8.14-rc:
$ ipmitool fru edit 0 field c 1 testtest
Read FRU Area length 712 too large, Adjusting to 184
Field not found !

Note: the "FRU Area length" value changes from one invocation to the next and appears to be somewhat random (does not actually come from the FRU area length reported by the BMC) and the "Field not found !" error means the edit command does not work.

Related

Bugs (use GitHub instead): #308

Discussion

  • Rob Swindell

    Rob Swindell - 2014-04-18

    The "FRU Area length" warning appears to be the result of this typo in read_fru_area():

                        /* subtract 1 byte for bytes count */
                        fru->max_write_size = max_rs_size - 1;
    

    That (apparently) should read:

                        fru->max_read_size = max_rs_size - 1;
    

    This does not fix the "Field not found !" error however.

     
  • Zdenek Styblik

    Zdenek Styblik - 2014-04-18

    Hrm, ok. Please, attach a diff/patch and I'll give it a commit.

     
  • Zdenek Styblik

    Zdenek Styblik - 2014-04-18
    • status: open --> open-accepted
    • assigned_to: Zdenek Styblik
     
    • Rob Swindell

      Rob Swindell - 2014-04-19

      I'll try to find all the causes of this (these) new bug[s] with the "fru edit" command, but why don't you ask Dmitry Bazhenov to fix this since he broke it with the "optimizations as well as several bug-fixes" that he introduced (and you committed) in CVS revision 1.84 of ipmi_fru.c?

      http://ipmitool.cvs.sourceforge.net/viewvc/ipmitool/ipmitool/lib/ipmi_fru.c?r1=1.83&r2=1.84

      -Rob

      From: Zdenek Styblik [mailto:stybla@users.sf.net]
      Sent: Thursday, April 17, 2014 9:42 PM
      To: [ipmitool:bugs]
      Subject: [ipmitool:bugs] #308 "fru edit" no longer works for non-zero fields

      • status: open --> open-accepted
      • assigned_to: Zdenek Styblik
      • Comment:

      Hrm, ok. Please, attach a diff/patch and I'll give it a commit.


      [bugs:#308]http://sourceforge.net/p/ipmitool/bugs/308/ "fru edit" no longer works for non-zero fields

      Status: open-accepted
      Group: version-1.8.14
      Created: Fri Apr 18, 2014 03:08 AM UTC by Rob Swindell
      Last Updated: Fri Apr 18, 2014 03:28 AM UTC
      Owner: Zdenek Styblik

      The "fru edit" command has a couple of regressions since v1.8.13.

      Using v1.8.13:
      $ ipmitool fru edit 0 field c 1 testtest
      Updating Field 'testtest' with 'testtest' ...

      Using v1.8.14-rc:
      $ ipmitool fru edit 0 field c 1 testtest
      Read FRU Area length 712 too large, Adjusting to 184
      Field not found !

      Note: the "FRU Area length" value changes from one invocation to the next and appears to be somewhat random (does not actually come from the FRU area length reported by the BMC) and the "Field not found !" error means the edit command does not work.


      Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/ipmitool/bugs/308/

      To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/

       

      Related

      Bugs (use GitHub instead): #308

  • Rob Swindell

    Rob Swindell - 2014-04-19

    Attached patch fixes various problems with the "fru edit" command introduced in CVS revision 1.84 of lib/ipmi_fru.c:

    1. Warnings about "FRU Area Length" based on uninitialized (malloc'd) memory contents (due to fru->max_read_size not being initialized, left at 0) and fru_data not being zeroed after malloc() in ipmi_fru_set_field_string().

    2. "fru edit" commands for any field index other than 0 would fail (with "Field not found !" error) due to a couple offset and length calculation errors (for all the supported "area" types) in ipmi_fru_set_field_string().

    3. "fru edit" commands would corrupt the FRU Inventory Area due to incorrect "source offset" value being specified in write_fru_area() call in impi_fru_set_field_string().

     
  • Zdenek Styblik

    Zdenek Styblik - 2014-04-25
    • status: open-accepted --> closed-fixed
     
    • Rob Swindell

      Rob Swindell - 2014-04-25

      There's a minor difference in the patch committed from the patch I submitted:

                  /* Write the updated section to the FRU data */
      
      • if( write_fru_area(intf, &fru, fruId, header_offset,
      • if( write_fru_area(intf, &fru, fruId, / source offset: / 0,
        header_offset, fru_section_len, fru_data) < 0 )
        {
        printf("Write to FRU data failed.\n");

      The embedded comment there ("source offset:") is describing the purpose of the constant '0' argument value to write_fru_area(). The committed patch merges this new comment with the preexisting comment before it, where it makes no sense. Of course this has no effect on the compiled object, but leaves the source in a less intelligible state than before. If you don't like the embedded comment, I suggest just removing it (rather than moving it to a context where it doesn't apply).

      As for your editorial comments, I wouldn't presume that anyone submitting bug reports or patches to ipmitool can put "work on ipmitool" in any weekly report or work sheet. Perhaps some can or do, but I certainly do not and would not. I'm only submitting patches because I have the know-how, the means, and (occasionally) the time. When I come across a ipmitool defect while attempting to assist our customers with ipmitool-related issues, I feel compelled to report them to the ipmitool project team and if time allows, fix them myself and submit a patch. This certainly does not fall under my job description or constitute my "work" and I wouldn't expect others to do anything "for me". The fix in question wasn't "for me", it was for ipmitool. I certainly have no dependency on the functionality of ipmitool. In the cases where a regression is found in ipmitool, it would be easier for me to just to tell our customers to use an older version of ipmitool or better yet, use the Free-IPMI suite of tools instead, but I feel compelled to help the ipmitool project and this comes across to you as my "work". I assure you, it is not: no one has ever directed or instructed me or my team members to fix anything in ipmitool.

      You've stated several times that you don't have IPMI cable hardware. Are you planning on remedying that situation? IPMI capable systems can be had very cheaply, and purely software IPMI implementations also exist (with limited or emulated functionality). If you have no plans to actually use or test ipmitool, I wonder why you're even involved in the project.

      -Rob

      From: Zdenek Styblik [mailto:stybla@users.sf.net]
      Sent: Friday, April 25, 2014 11:59 AM
      To: [ipmitool:bugs]
      Subject: [ipmitool:bugs] #308 "fru edit" no longer works for non-zero fields

      • status: open-accepted --> closed-fixed
      • Comment:

      I've committed your patch into Git. As there is no RC3 planned, please, test with tip of(HEAD) the Git.

      Also Rob, don't expect others to do work for you. There are two things I could do and would have done. First, look at it later. Second, revert Dmitry's commit. And that's about it.
      I'm also fairly sure you could have e-mailed Dmitry yourself. And unlike you, I can't put "work" on ipmitool into my weekly report(work sheet). Not to mention I don't have access to IPMI capable HW anymore.

      Thanks for the report and patch.


      [bugs:#308]http://sourceforge.net/p/ipmitool/bugs/308/ "fru edit" no longer works for non-zero fields

      Status: closed-fixed
      Group: version-1.8.14
      Created: Fri Apr 18, 2014 03:08 AM UTC by Rob Swindell
      Last Updated: Sat Apr 19, 2014 01:50 AM UTC
      Owner: Zdenek Styblik

      The "fru edit" command has a couple of regressions since v1.8.13.

      Using v1.8.13:
      $ ipmitool fru edit 0 field c 1 testtest
      Updating Field 'testtest' with 'testtest' ...

      Using v1.8.14-rc:
      $ ipmitool fru edit 0 field c 1 testtest
      Read FRU Area length 712 too large, Adjusting to 184
      Field not found !

      Note: the "FRU Area length" value changes from one invocation to the next and appears to be somewhat random (does not actually come from the FRU area length reported by the BMC) and the "Field not found !" error means the edit command does not work.


      Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/ipmitool/bugs/308/

      To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/

       

      Related

      Bugs (use GitHub instead): #308

      • Zdenek Styblik

        Zdenek Styblik - 2014-04-26

        The embedded comment there ("source offset:") is describing the purpose of the constant '0' argument value to write_fru_area(). The committed patch merges this new comment with the preexisting comment before it, where it makes no sense. Of course this has no effect on the compiled object, but leaves the source in a less intelligible state than before. If you don't like the embedded comment, I suggest just removing it (rather than moving it to a context where it doesn't apply).

        Yes, I don't like embedded comments. But the end result really isn't what I wanted. I meant something like:

        /* Write the updated section to the FRU data; source offset => 0 */
        

        Other option would be to use variable to be even more explicit:

        source_offset = 0;
        

        As for your editorial comments, I wouldn't presume that anyone submitting bug reports or patches to ipmitool can put "work on ipmitool" in any weekly report or work sheet. Perhaps some can or do, but I certainly do not and would not.

        Yes, some people are actually paid to work on ipmitool. However, they only work at what is in interest of their employer. And overall project health isn't it. That's how it goes nowadays.
        Ad what to tell to customers, sure. They can get involved, they can use something else, freedom of choice. Sorry, I'm not going to convince anybody one way or another.
        Anyway, I don't understand why you've said what you did then.

        Are you planning on remedying that situation? IPMI capable systems can be had very cheaply

        If you mean buying physical server with my own money, then the answer is no. Not even to have one at home, to be honest. I would get evicted soon because of the noise(produced/emitted by server).

        and purely software IPMI implementations also exist (with limited or emulated functionality)

        I'm not aware of these implementations. Yes, libopenipmi supposedly can simulate some things.

        If you have no plans to actually use or test ipmitool,

        This is rather question of resources and priorities. So either the question or implication is wrong.
        Either way, I have only so much of free time I can put in.

        I wonder why you're even involved in the project.

        Because there is nobody else?

         
        • Rob Swindell

          Rob Swindell - 2014-04-28

          This is not an endorsement, but I can say that there are IPMI-capable servers which are both cheap and quiet. For example:
          http://www.amazon.com/Supermicro-SuperServer-Rackmount-Barebone-SYS-5015A-EHF-D525/dp/B004GKULFO

          -Rob

          From: Zdenek Styblik [mailto:stybla@users.sf.net]
          Sent: Friday, April 25, 2014 9:58 PM
          To: [ipmitool:bugs]
          Subject: [ipmitool:bugs] Re: #308 "fru edit" no longer works for non-zero fields

          The embedded comment there ("source offset:") is describing the purpose of the constant '0' argument value to write_fru_area(). The committed patch merges this new comment with the preexisting comment before it, where it makes no sense. Of course this has no effect on the compiled object, but leaves the source in a less intelligible state than before. If you don't like the embedded comment, I suggest just removing it (rather than moving it to a context where it doesn't apply).

          Yes, I don't like embedded comments. But the end result really isn't what I wanted. I meant something like:

          / Write the updated section to the FRU data; source offset => 0 /

          Other option would be to use variable to be even more explicit:

          source_offset = 0;

          As for your editorial comments, I wouldn't presume that anyone submitting bug reports or patches to ipmitool can put "work on ipmitool" in any weekly report or work sheet. Perhaps some can or do, but I certainly do not and would not.

          Yes, some people are actually paid to work on ipmitool. However, they only work at what is in interest of their employer. And overall project health isn't it. That's how it goes nowadays.
          Ad what to tell to customers, sure. They can get involved, they can use something else, freedom of choice. Sorry, I'm not going to convince anybody one way or another.
          Anyway, I don't understand why you've said what you did then.

          Are you planning on remedying that situation? IPMI capable systems can be had very cheaply

          If you mean buying physical server with my own money, then the answer is no. Not even to have one at home, to be honest. I would get evicted soon because of the noise(produced/emitted by server).

          and purely software IPMI implementations also exist (with limited or emulated functionality)

          I'm not aware of these implementations. Yes, libopenipmi supposedly can simulate some things.

          If you have no plans to actually use or test ipmitool,

          This is rather question of resources and priorities. So either the question or implication is wrong.
          Either way, I have only so much of free time I can put in.

          I wonder why you're even involved in the project.

          Because there is nobody else?


          [bugs:#308]http://sourceforge.net/p/ipmitool/bugs/308/ "fru edit" no longer works for non-zero fields

          Status: closed-fixed
          Group: version-1.8.14
          Created: Fri Apr 18, 2014 03:08 AM UTC by Rob Swindell
          Last Updated: Fri Apr 25, 2014 06:59 PM UTC
          Owner: Zdenek Styblik

          The "fru edit" command has a couple of regressions since v1.8.13.

          Using v1.8.13:
          $ ipmitool fru edit 0 field c 1 testtest
          Updating Field 'testtest' with 'testtest' ...

          Using v1.8.14-rc:
          $ ipmitool fru edit 0 field c 1 testtest
          Read FRU Area length 712 too large, Adjusting to 184
          Field not found !

          Note: the "FRU Area length" value changes from one invocation to the next and appears to be somewhat random (does not actually come from the FRU area length reported by the BMC) and the "Field not found !" error means the edit command does not work.


          Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/ipmitool/bugs/308/

          To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/

           

          Related

          Bugs (use GitHub instead): #308

  • Zdenek Styblik

    Zdenek Styblik - 2014-04-25

    I've committed your patch into Git. As there is no RC3 planned, please, test with tip of(HEAD) the Git.

    Also Rob, don't expect others to do work for you. There are two things I could do and would have done. First, look at it later. Second, revert Dmitry's commit. And that's about it.
    I'm also fairly sure you could have e-mailed Dmitry yourself. And unlike you, I can't put "work" on ipmitool into my weekly report(work sheet). Not to mention I don't have access to IPMI capable HW anymore.

    Thanks for the report and patch.

     

Log in to post a comment.

MongoDB Logo MongoDB