On 03/17/2010 10:13 AM, Daniel Veillard wrote:
On Mon, Mar 15, 2010 at 10:13:30PM -0400, David Allan wrote:
---
src/storage/storage_driver.c | 224 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 224 insertions(+), 0 deletions(-)
[...]
+ char errbuf[64];
While the 1024 used many places in the code is probably a bit too
much, 64 chars might be a bit small for an errno return description
I would bump it to 128,
Actually, you shouldn't need the errbuf at all - see my comments on the
use of errbuf below.
+ ret = ftruncate(fd, 0);
+ if (ret == -1) {
+ virReportSystemError(ret,
+ _("Failed to truncate volume with "
+ "path '%s' to 0 bytes: '%s'"),
+ vol->target.path,
+ virStrerror(errno, errbuf, sizeof(errbuf)));
Since ftruncate returns a -1 on failure, and you're sending that return
value to virRerportSystemError (which would barf on the -1) and then
manually printing out the strerror(). Instead, this should be changed to:
virReportSystemError(errno,
_("Failed to truncate volume with "
"path '%s' to 0 bytes"),
vol->target.path);
+ goto out;
+ }
+
+ ret = ftruncate(fd, size);
+ if (ret == -1) {
+ virReportSystemError(ret,
+ _("Failed to truncate volume with "
+ "path '%s' to %ju bytes: '%s'\n"),
+ vol->target.path, (intmax_t)size,
+ virStrerror(errno, errbuf, sizeof(errbuf)));
Likewise, this should be:
virReportSystemError(errno,
_("Failed to truncate volume with "
"path '%s' to %ju bytes),
vol->target.path, (intmax_t)size);
virStrerror(errno, errbuf, sizeof(errbuf)));
+ }
+
+out:
+ return ret;
+}
+
+
+static int
+storageWipeExtent(virStorageVolDefPtr vol,
+ int fd,
+ off_t extent_start,
+ off_t extent_length,
+ char *writebuf,
+ size_t writebuf_length,
+ size_t *bytes_wiped)
+{
+ int ret = -1, written = 0;
no need to initialize ret here
+ off_t remaining = 0;
+ size_t write_size = 0;
+ char errbuf[64];
same 128 vs 64
Again, you shouldn't need errbuf at all.
+ VIR_DEBUG("extent logical start: %ju len: %ju",
+ (intmax_t)extent_start, (intmax_t)extent_length);
+
+ if ((ret = lseek(fd, extent_start, SEEK_SET))< 0) {
+ virReportSystemError(ret,
+ _("Failed to seek to position %ju in volume "
+ "with path '%s': '%s'"),
+ (intmax_t)extent_start, vol->target.path,
+ virStrerror(errno, errbuf, sizeof(errbuf)));
Same comment: lseek returns -1, not an errno. replace "ret" with errno,
remove the final %s, and get rid of the virStrerror() arg.
+ goto out;
+ }
+
+ remaining = extent_length;
+ while (remaining> 0) {
+
+ write_size = (writebuf_length< remaining) ? writebuf_length : remaining;
+ written = safewrite(fd, writebuf, write_size);
+ if (written< 0) {
+ virReportSystemError(written,
+ _("Failed to write to storage volume with "
+ "path '%s': '%s' "
+ "(attempted to write %zu bytes)"),
+ vol->target.path,
+ virStrerror(errno, errbuf, sizeof(errbuf)),
+ write_size);
Again - safewrite returns # of bytes written, or -1, not an errno.
Replace "written" with errno, and get rid of virStrError() and the extra %s.
+ goto out;
+ }
+
+ *bytes_wiped += written;
+ remaining -= written;
+ }
+
+ VIR_DEBUG("Wrote %zu bytes to volume with path '%s'",
+ *bytes_wiped, vol->target.path);
+
+ ret = 0;
+
+out:
+ return ret;
+}
+
+
+static int
+storageVolumeWipeInternal(virStorageVolDefPtr def)
+{
+ int ret = -1, fd = -1;
+ char errbuf[64];
You don't need errbuf.
+ struct stat st;
+ char *writebuf = NULL;
+ size_t bytes_wiped = 0;
+
+ VIR_DEBUG("Wiping volume with path '%s'", def->target.path);
+
+ fd = open(def->target.path, O_RDWR);
+ if (fd == -1) {
+ VIR_ERROR("Failed to open storage volume with path '%s': '%s'",
+ def->target.path,
+ virStrerror(errno, errbuf, sizeof(errbuf)));
Not sure why you're using VIR_ERROR() + manually adding virStrerror() -
isn't this the same thing as virReportSystemError?
+ goto out;
+ }
+
+ if (fstat(fd,&st) == -1) {
+ VIR_ERROR("Failed to stat storage volume with path '%s': '%s'",
+ def->target.path,
+ virStrerror(errno, errbuf, sizeof(errbuf)));
And again.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list