Project

General

Profile

Bug #8399

zeroing of end of disk needs a count

Added by Frank Riley over 5 years ago. Updated about 3 years ago.

Status:
Closed: Not To Be Fixed
Priority:
No priority
Assignee:
William Grzybowski
Category:
OS
Target version:
Seen in:
Severity:
New
Reason for Closing:
Reason for Blocked:
Needs QA:
Yes
Needs Doc:
Yes
Needs Merging:
Yes
Needs Automation:
No
Support Suite Ticket:
n/a
Hardware Configuration:
ChangeLog Required:
No

Description

There are several places in notifier.py that do the following:

size = get size of disk
...
# HACK: force the wipe at the end of the disk to always succeed. This
# is a lame workaround.
self._system("dd if=/dev/zero of=/dev/%s bs=1m oseek=%s" % (
                devname,
                size / 1024 - 4,
                ))

Since count is not specified, it happily tries to continue writing past the end of the disk, which results in an error. Setting count=4 fixes this.

History

#1 Updated by Jordan Hubbard over 5 years ago

  • Assignee set to William Grzybowski
  • Target version set to Unspecified

#2 Updated by William Grzybowski over 5 years ago

Why is that a problem? we dont check for exit status code?

#3 Updated by William Grzybowski over 5 years ago

  • Status changed from Unscreened to Screened

#4 Updated by Frank Riley over 5 years ago

The comment indicates the author wanted to check the return status, but didn't know why it was failing. I think this code is clearing the secondary GPT info. Should the code fail if that can't be cleared?

#5 Updated by William Grzybowski over 5 years ago

Looking at the git log, you can see this:

+        # HACK: force the wipe at the end of the disk to always succeed. This
+        # is a lame workaround.
         self.__system("dd if=/dev/zero of=/dev/%s bs=1m oseek=`diskinfo %s " 
-                      "| awk '{print int($3 / (1024*1024)) - 4;}'`" % (devname, devname))
+                      "| awk '{print int($3 / (1024*1024)) - 4;}'` || :" % (devname, devname))

The real reason for this comment was the "|| :" to make sure the command returns true, because for some reason there was some check to ensure it succeeded.

I don't think this is longer the case.

#6 Updated by Frank Riley over 5 years ago

Well, since the system call return status isn't checked, I don't see any reason to force a return status of true. The comment to me indicated the status mattered and could be returned correctly by adding the count parameter. If the intent was to ignore the status anyway, then the comment doesn't apply. I put this in as low priority because I couldn't discern what the intent was and figured someone on your end would know whether this is something to care about. If it's not, no big deal.

#7 Updated by William Grzybowski over 5 years ago

  • Status changed from Screened to Closed: Not To Be Fixed

It is not, thanks for pointing it out though.

#8 Avatar?id=14398&size=24x24 Updated by Kris Moore about 3 years ago

  • Category set to 129
  • Target version changed from Unspecified to N/A
  • Seen in changed from to N/A

Also available in: Atom PDF