Add bool 'zero_end' and logic that would allow a caller to wipe specific
portions of a target device either from the beginning (the default) or
from the end when zero_end is true.

This will allow for this code to wipe out partition table information
from a device.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/storage/storage_util.c | 48 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index a2d89af..c1734e7 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2516,24 +2516,44 @@ static int
 storageBackendWipeLocal(const char *path,
                         int fd,
                         unsigned long long wipe_len,
-                        size_t writebuf_length)
+                        size_t writebuf_length,
+                        bool zero_end)
 {
     int ret = -1, written = 0;
     unsigned long long remaining = 0;
+    off_t size;
     size_t write_size = 0;
     char *writebuf = NULL;
 
-    VIR_DEBUG("wiping start: 0 len: %llu", wipe_len);
-
     if (VIR_ALLOC_N(writebuf, writebuf_length) < 0)
         goto cleanup;
 
-    if (lseek(fd, 0, SEEK_SET) < 0) {
-        virReportSystemError(errno,
-                             _("Failed to seek to the start in volume "
-                               "with path '%s'"),
-                             path);
-        goto cleanup;
+    if (!zero_end) {
+        if (lseek(fd, 0, SEEK_SET) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to seek to the start in volume "
+                                   "with path '%s'"),
+                                 path);
+            goto cleanup;
+        }
+        VIR_DEBUG("wiping start: 0 len: %llu", wipe_len);
+    } else {
+        if ((size = lseek(fd, 0, SEEK_END)) == (off_t)-1) {
+            virReportSystemError(errno,
+                                 _("Failed to seek to the end in volume "
+                                   "with path '%s'"),
+                                 path);
+            goto cleanup;
+        }
+        size -= wipe_len;
+        if (lseek(fd, size, SEEK_SET) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to seek to %zd bytes in volume "
+                                   "with path '%s'"),
+                                 size, path);
+            goto cleanup;
+        }
+        VIR_DEBUG("wiping start: %zd len: %llu", size, wipe_len);
     }
 
     remaining = wipe_len;
@@ -2573,7 +2593,8 @@ storageBackendWipeLocal(const char *path,
 static int
 storageBackendVolWipeLocalFile(const char *path,
                                unsigned int algorithm,
-                               unsigned long long allocation)
+                               unsigned long long allocation,
+                               bool zero_end)
 {
     int ret = -1, fd = -1;
     const char *alg_char = NULL;
@@ -2648,7 +2669,8 @@ storageBackendVolWipeLocalFile(const char *path,
         if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) {
             ret = storageBackendVolZeroSparseFileLocal(path, st.st_size, fd);
         } else {
-            ret = storageBackendWipeLocal(path, fd, allocation, st.st_blksize);
+            ret = storageBackendWipeLocal(path, fd, allocation, st.st_blksize,
+                                          zero_end);
         }
         if (ret < 0)
             goto cleanup;
@@ -2686,7 +2708,7 @@ storageBackendVolWipePloop(virStorageVolDefPtr vol,
         goto cleanup;
 
     if (storageBackendVolWipeLocalFile(target_path, algorithm,
-                                       vol->target.allocation) < 0)
+                                       vol->target.allocation, false) < 0)
         goto cleanup;
 
     if (virFileRemove(disk_desc, 0, 0) < 0) {
@@ -2735,7 +2757,7 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
         ret = storageBackendVolWipePloop(vol, algorithm);
     } else {
         ret = storageBackendVolWipeLocalFile(vol->target.path, algorithm,
-                                             vol->target.allocation);
+                                             vol->target.allocation, false);
     }
 
     return ret;
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 04/07/2017 06:30 PM, John Ferlan wrote:
> Add bool 'zero_end' and logic that would allow a caller to wipe specific
> portions of a target device either from the beginning (the default) or
> from the end when zero_end is true.
> 
> This will allow for this code to wipe out partition table information
> from a device.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/storage/storage_util.c | 48 +++++++++++++++++++++++++++++++++-------------
>  1 file changed, 35 insertions(+), 13 deletions(-)
> 
> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
> index a2d89af..c1734e7 100644
> --- a/src/storage/storage_util.c
> +++ b/src/storage/storage_util.c
> @@ -2516,24 +2516,44 @@ static int
>  storageBackendWipeLocal(const char *path,
>                          int fd,
>                          unsigned long long wipe_len,
> -                        size_t writebuf_length)
> +                        size_t writebuf_length,
> +                        bool zero_end)
>  {
>      int ret = -1, written = 0;
>      unsigned long long remaining = 0;
> +    off_t size;
>      size_t write_size = 0;
>      char *writebuf = NULL;
>  
> -    VIR_DEBUG("wiping start: 0 len: %llu", wipe_len);
> -
>      if (VIR_ALLOC_N(writebuf, writebuf_length) < 0)
>          goto cleanup;
>  
> -    if (lseek(fd, 0, SEEK_SET) < 0) {
> -        virReportSystemError(errno,
> -                             _("Failed to seek to the start in volume "
> -                               "with path '%s'"),
> -                             path);
> -        goto cleanup;
> +    if (!zero_end) {
> +        if (lseek(fd, 0, SEEK_SET) < 0) {
> +            virReportSystemError(errno,
> +                                 _("Failed to seek to the start in volume "
> +                                   "with path '%s'"),
> +                                 path);
> +            goto cleanup;
> +        }
> +        VIR_DEBUG("wiping start: 0 len: %llu", wipe_len);
> +    } else {
> +        if ((size = lseek(fd, 0, SEEK_END)) == (off_t)-1) {
> +            virReportSystemError(errno,
> +                                 _("Failed to seek to the end in volume "
> +                                   "with path '%s'"),
> +                                 path);
> +            goto cleanup;
> +        }
> +        size -= wipe_len;
> +        if (lseek(fd, size, SEEK_SET) < 0) {
> +            virReportSystemError(errno,
> +                                 _("Failed to seek to %zd bytes in volume "
> +                                   "with path '%s'"),
> +                                 size, path);

Is off_t really ssize_t? I think we should typecast it.

> +            goto cleanup;
> +        }

Or, instead of these two seeks:

if ((size  = lseek(fd, -wipe_len, SEEK_END)) < 0) {
  virReportSystemError();
  goto cleanup;
}

> +        VIR_DEBUG("wiping start: %zd len: %llu", size, wipe_len);

This DEBUG is the same as in the other body for the if statement. While
it's just VIR_DEBUG I wouldn't care, but just consider the following
for a moment:

Move this VIR_DEBUG right after this if statement and initialize @size
to zero (and obviously drop the other VIR_DEBUG with hardcoded 0). That
way we know what the current position is and how long section is to be
wiped.

Or even better:

+    if (!zero_end) {
+        if ((size = lseek(fd, 0, SEEK_SET)) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to seek to the start in volume "
+                                   "with path '%s'"),
+                                 path);
+            goto cleanup;
+        }
+    } else {
+        if ((size = lseek(fd, -wipe_len, SEEK_END)) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to seek to %llu bytes to the end in volume "
+                                   "with path '%s'"),
+                                 wipe_len, path);
+            goto cleanup;
+        }
     }
 
+    VIR_DEBUG("wiping start: %zd len: %llu", (ssize_t) size, wipe_len);


>      }
>  
>      remaining = wipe_len;
> @@ -2573,7 +2593,8 @@ storageBackendWipeLocal(const char *path,
>  static int
>  storageBackendVolWipeLocalFile(const char *path,
>                                 unsigned int algorithm,
> -                               unsigned long long allocation)
> +                               unsigned long long allocation,
> +                               bool zero_end)
>  {
>      int ret = -1, fd = -1;
>      const char *alg_char = NULL;
> @@ -2648,7 +2669,8 @@ storageBackendVolWipeLocalFile(const char *path,
>          if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) {
>              ret = storageBackendVolZeroSparseFileLocal(path, st.st_size, fd);
>          } else {
> -            ret = storageBackendWipeLocal(path, fd, allocation, st.st_blksize);
> +            ret = storageBackendWipeLocal(path, fd, allocation, st.st_blksize,
> +                                          zero_end);
>          }
>          if (ret < 0)
>              goto cleanup;
> @@ -2686,7 +2708,7 @@ storageBackendVolWipePloop(virStorageVolDefPtr vol,
>          goto cleanup;
>  
>      if (storageBackendVolWipeLocalFile(target_path, algorithm,
> -                                       vol->target.allocation) < 0)
> +                                       vol->target.allocation, false) < 0)
>          goto cleanup;
>  
>      if (virFileRemove(disk_desc, 0, 0) < 0) {
> @@ -2735,7 +2757,7 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
>          ret = storageBackendVolWipePloop(vol, algorithm);
>      } else {
>          ret = storageBackendVolWipeLocalFile(vol->target.path, algorithm,
> -                                             vol->target.allocation);
> +                                             vol->target.allocation, false);
>      }
>  
>      return ret;
> 

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 04/25/2017 03:06 AM, Michal Privoznik wrote:
> On 04/07/2017 06:30 PM, John Ferlan wrote:
>> Add bool 'zero_end' and logic that would allow a caller to wipe specific
>> portions of a target device either from the beginning (the default) or
>> from the end when zero_end is true.
>>
>> This will allow for this code to wipe out partition table information
>> from a device.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>>  src/storage/storage_util.c | 48 +++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 35 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
>> index a2d89af..c1734e7 100644
>> --- a/src/storage/storage_util.c
>> +++ b/src/storage/storage_util.c
>> @@ -2516,24 +2516,44 @@ static int
>>  storageBackendWipeLocal(const char *path,
>>                          int fd,
>>                          unsigned long long wipe_len,
>> -                        size_t writebuf_length)
>> +                        size_t writebuf_length,
>> +                        bool zero_end)
>>  {
>>      int ret = -1, written = 0;
>>      unsigned long long remaining = 0;
>> +    off_t size;
>>      size_t write_size = 0;
>>      char *writebuf = NULL;
>>  
>> -    VIR_DEBUG("wiping start: 0 len: %llu", wipe_len);
>> -
>>      if (VIR_ALLOC_N(writebuf, writebuf_length) < 0)
>>          goto cleanup;
>>  
>> -    if (lseek(fd, 0, SEEK_SET) < 0) {
>> -        virReportSystemError(errno,
>> -                             _("Failed to seek to the start in volume "
>> -                               "with path '%s'"),
>> -                             path);
>> -        goto cleanup;
>> +    if (!zero_end) {
>> +        if (lseek(fd, 0, SEEK_SET) < 0) {
>> +            virReportSystemError(errno,
>> +                                 _("Failed to seek to the start in volume "
>> +                                   "with path '%s'"),
>> +                                 path);
>> +            goto cleanup;
>> +        }
>> +        VIR_DEBUG("wiping start: 0 len: %llu", wipe_len);
>> +    } else {
>> +        if ((size = lseek(fd, 0, SEEK_END)) == (off_t)-1) {
>> +            virReportSystemError(errno,
>> +                                 _("Failed to seek to the end in volume "
>> +                                   "with path '%s'"),
>> +                                 path);
>> +            goto cleanup;
>> +        }
>> +        size -= wipe_len;
>> +        if (lseek(fd, size, SEEK_SET) < 0) {
>> +            virReportSystemError(errno,
>> +                                 _("Failed to seek to %zd bytes in volume "
>> +                                   "with path '%s'"),
>> +                                 size, path);
> 
> Is off_t really ssize_t? I think we should typecast it.
> 

OK - I do (now) have a recollection of printing an off_t cause some sort
of issue for some arch, but I forget which one...

>> +            goto cleanup;
>> +        }
> 
> Or, instead of these two seeks:
> 
> if ((size  = lseek(fd, -wipe_len, SEEK_END)) < 0) {
>   virReportSystemError();
>   goto cleanup;
> }
> 
>> +        VIR_DEBUG("wiping start: %zd len: %llu", size, wipe_len);
> 
> This DEBUG is the same as in the other body for the if statement. While
> it's just VIR_DEBUG I wouldn't care, but just consider the following
> for a moment:
> 
> Move this VIR_DEBUG right after this if statement and initialize @size
> to zero (and obviously drop the other VIR_DEBUG with hardcoded 0). That
> way we know what the current position is and how long section is to be
> wiped.
> 
> Or even better:
> 
> +    if (!zero_end) {
> +        if ((size = lseek(fd, 0, SEEK_SET)) < 0) {
> +            virReportSystemError(errno,
> +                                 _("Failed to seek to the start in volume "
> +                                   "with path '%s'"),
> +                                 path);
> +            goto cleanup;
> +        }
> +    } else {
> +        if ((size = lseek(fd, -wipe_len, SEEK_END)) < 0) {
> +            virReportSystemError(errno,
> +                                 _("Failed to seek to %llu bytes to the end in volume "
> +                                   "with path '%s'"),
> +                                 wipe_len, path);
> +            goto cleanup;
> +        }
>      }
>  
> +    VIR_DEBUG("wiping start: %zd len: %llu", (ssize_t) size, wipe_len);
> 
> 

I'll go with this...

Thanks

John

>>      }
>>  
>>      remaining = wipe_len;
>> @@ -2573,7 +2593,8 @@ storageBackendWipeLocal(const char *path,
>>  static int
>>  storageBackendVolWipeLocalFile(const char *path,
>>                                 unsigned int algorithm,
>> -                               unsigned long long allocation)
>> +                               unsigned long long allocation,
>> +                               bool zero_end)
>>  {
>>      int ret = -1, fd = -1;
>>      const char *alg_char = NULL;
>> @@ -2648,7 +2669,8 @@ storageBackendVolWipeLocalFile(const char *path,
>>          if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) {
>>              ret = storageBackendVolZeroSparseFileLocal(path, st.st_size, fd);
>>          } else {
>> -            ret = storageBackendWipeLocal(path, fd, allocation, st.st_blksize);
>> +            ret = storageBackendWipeLocal(path, fd, allocation, st.st_blksize,
>> +                                          zero_end);
>>          }
>>          if (ret < 0)
>>              goto cleanup;
>> @@ -2686,7 +2708,7 @@ storageBackendVolWipePloop(virStorageVolDefPtr vol,
>>          goto cleanup;
>>  
>>      if (storageBackendVolWipeLocalFile(target_path, algorithm,
>> -                                       vol->target.allocation) < 0)
>> +                                       vol->target.allocation, false) < 0)
>>          goto cleanup;
>>  
>>      if (virFileRemove(disk_desc, 0, 0) < 0) {
>> @@ -2735,7 +2757,7 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
>>          ret = storageBackendVolWipePloop(vol, algorithm);
>>      } else {
>>          ret = storageBackendVolWipeLocalFile(vol->target.path, algorithm,
>> -                                             vol->target.allocation);
>> +                                             vol->target.allocation, false);
>>      }
>>  
>>      return ret;
>>
> 
> Michal
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list