qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] Wipe patch
@ 2006-07-26  8:23 ZIGLIO, Frediano, VF-IT
  2006-07-26  8:41 ` Brad Campbell
  2006-08-02  2:25 ` Brad Campbell
  0 siblings, 2 replies; 14+ messages in thread
From: ZIGLIO, Frediano, VF-IT @ 2006-07-26  8:23 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

Hi,
  well, this is not a definitive patch but it works. The aim is to be
able to wipe the disk without allocating entire space. When you wipe a
disk the program fill disk with zero bytes so disk image increase to
allocate all space. This just patch detect null byte writes and do not
write all zero byte clusters.

Frediano Ziglio


[-- Attachment #2: wipe.diff --]
[-- Type: application/octet-stream, Size: 2580 bytes --]

Index: block-qcow.c
===================================================================
RCS file: /sources/qemu/qemu/block-qcow.c,v
retrieving revision 1.7
diff -u -1 -0 -r1.7 block-qcow.c
--- block-qcow.c	4 Jun 2006 11:39:06 -0000	1.7
+++ block-qcow.c	26 Jul 2006 07:15:17 -0000
@@ -480,47 +480,77 @@
                                 &s->aes_decrypt_key);
             }
         }
         nb_sectors -= n;
         sector_num += n;
         buf += n * 512;
     }
     return 0;
 }
 
+static int is_not_zero(const uint8_t *data, int len)
+{
+    int left;
+    while (len && (((unsigned int) data) & 3) != 0) {
+        if (*data++)
+            return 1;
+        --len;
+    }
+    left = len & 3;
+    len >>= 2;
+    while (len) {
+        if (*((uint32_t *)data) != 0)
+            return 1;
+        data += 4;
+        --len;
+    }
+    while (left) {
+        if (*data++)
+            return 1;
+        --left;
+    }
+    return 0;
+}
+
 static int qcow_write(BlockDriverState *bs, int64_t sector_num, 
                      const uint8_t *buf, int nb_sectors)
 {
     BDRVQcowState *s = bs->opaque;
     int ret, index_in_cluster, n;
     uint64_t cluster_offset;
+    int not_zero;
     
     while (nb_sectors > 0) {
         index_in_cluster = sector_num & (s->cluster_sectors - 1);
         n = s->cluster_sectors - index_in_cluster;
         if (n > nb_sectors)
             n = nb_sectors;
-        cluster_offset = get_cluster_offset(bs, sector_num << 9, 1, 0, 
+        not_zero = is_not_zero(buf, n * 512);
+        cluster_offset = get_cluster_offset(bs, sector_num << 9, not_zero, 0, 
                                             index_in_cluster, 
                                             index_in_cluster + n);
-        if (!cluster_offset)
-            return -1;
+        if (!cluster_offset) {
+            if (not_zero)
+                return -1;
+            goto next_cluster;
+        }
         lseek(s->fd, cluster_offset + index_in_cluster * 512, SEEK_SET);
         if (s->crypt_method) {
             encrypt_sectors(s, sector_num, s->cluster_data, buf, n, 1,
                             &s->aes_encrypt_key);
             ret = write(s->fd, s->cluster_data, n * 512);
         } else {
             ret = write(s->fd, buf, n * 512);
         }
         if (ret != n * 512) 
             return -1;
+next_cluster:
         nb_sectors -= n;
         sector_num += n;
         buf += n * 512;
     }
     s->cluster_cache_offset = -1; /* disable compressed cache */
     return 0;
 }
 
 static void qcow_close(BlockDriverState *bs)
 {

^ permalink raw reply	[flat|nested] 14+ messages in thread
* RE: [Qemu-devel] Wipe patch
@ 2006-07-26  8:48 ZIGLIO, Frediano, VF-IT
  0 siblings, 0 replies; 14+ messages in thread
From: ZIGLIO, Frediano, VF-IT @ 2006-07-26  8:48 UTC (permalink / raw)
  To: qemu-devel

> 
> ZIGLIO, Frediano, VF-IT wrote:
> > Hi,
> >   well, this is not a definitive patch but it works. The 
> aim is to be
> > able to wipe the disk without allocating entire space. When 
> you wipe a
> > disk the program fill disk with zero bytes so disk image increase to
> > allocate all space. This just patch detect null byte writes 
> and do not
> > write all zero byte clusters.
> 
> This _looks_ like it would severely impact cpu load during a write.
> Have you done any testing to determine if this is likely to 
> impact a normal usage scenario?
> 
> Brad

No... as I said is not a definitive patch. I mainly wanted to see
comments on this stuff... I'm using it since some weeks and I didn't
note all that difference. Do you know any benchmark I could use?

Frediano

^ permalink raw reply	[flat|nested] 14+ messages in thread
* RE: [Qemu-devel] Wipe patch
@ 2006-07-26  9:42 ZIGLIO, Frediano, VF-IT
  2006-07-26  9:52 ` Brad Campbell
  0 siblings, 1 reply; 14+ messages in thread
From: ZIGLIO, Frediano, VF-IT @ 2006-07-26  9:42 UTC (permalink / raw)
  To: qemu-devel

> 
> Avi Kivity wrote:
> 
> >> This _looks_ like it would severely impact cpu load during a write.
> >> Have you done any testing to determine if this is likely 
> to impact a 
> >> normal usage scenario?
> > 
> > Why would it? In most cases, the zero test would terminate quickly, 
> > without accessing the entire cluster.
> > 
> 
> Good point, when I looked at it my brain was in zero wipe 
> mode.. which would be slow but then a 
> bucketload faster than writing the actual cluster to disk..
> 
> Brad

I use the patch to reduce image size. I use a RIP (repair is possible)
ISO image to boot a small Linux system where I can issue ntfswipe (or
any other wipe command). Than I can recompress image with qemu-img.

freddy77

^ permalink raw reply	[flat|nested] 14+ messages in thread
* RE: [Qemu-devel] Wipe patch
@ 2006-08-02 12:53 ZIGLIO, Frediano, VF-IT
  0 siblings, 0 replies; 14+ messages in thread
From: ZIGLIO, Frediano, VF-IT @ 2006-08-02 12:53 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1182 bytes --]

> 
> ZIGLIO, Frediano, VF-IT wrote:
> > Hi,
> >   well, this is not a definitive patch but it works. The 
> aim is to be
> > able to wipe the disk without allocating entire space. When 
> you wipe a
> > disk the program fill disk with zero bytes so disk image increase to
> > allocate all space. This just patch detect null byte writes 
> and do not
> > write all zero byte clusters.
> > 
> 
> I've been giving this some pretty heavy testing over the last 
> week and can say I've not noticed any 
> negative performance impact or any other adverse side 
> effects, not to mention the speedup when doing 
> re-packing (which I do fairly regularly on both ext3 and ntfs 
> guest filesystems).
> 
> While I'm here does anyone know of a simple program, either 
> dos or linux based for wiping unused 
> space on fat filesystems? The only ones I've found so far 
> have been windows based.
> 
> This patch now conflicts pretty heavily with the new AIO 
> changes it would seem. Further 
> investigation required.
> 
> Ta,
> Brad

Here you are updated patch. Current CVS seems to not compile on my
machine (but this is another problem...)

freddy77


[-- Attachment #2: wipe.diff --]
[-- Type: application/octet-stream, Size: 2578 bytes --]

Index: block-qcow.c
===================================================================
RCS file: /sources/qemu/qemu/block-qcow.c,v
retrieving revision 1.8
diff -u -1 -0 -r1.8 block-qcow.c
--- block-qcow.c	1 Aug 2006 16:21:11 -0000	1.8
+++ block-qcow.c	2 Aug 2006 12:48:48 -0000
@@ -480,47 +480,77 @@
             }
         }
         nb_sectors -= n;
         sector_num += n;
         buf += n * 512;
     }
     return 0;
 }
 #endif
 
+static int is_not_zero(const uint8_t *data, int len)
+{
+    int left;
+    while (len && (((unsigned int) data) & 3) != 0) {
+        if (*data++)
+            return 1;
+        --len;
+    }
+    left = len & 3;
+    len >>= 2;
+    while (len) {
+        if (*((uint32_t *)data) != 0)
+            return 1;
+        data += 4;
+        --len;
+    }
+    while (left) {
+        if (*data++)
+            return 1;
+        --left;
+    }
+    return 0;
+}
+
 static int qcow_write(BlockDriverState *bs, int64_t sector_num, 
                      const uint8_t *buf, int nb_sectors)
 {
     BDRVQcowState *s = bs->opaque;
     int ret, index_in_cluster, n;
     uint64_t cluster_offset;
+    int not_zero;
     
     while (nb_sectors > 0) {
         index_in_cluster = sector_num & (s->cluster_sectors - 1);
         n = s->cluster_sectors - index_in_cluster;
         if (n > nb_sectors)
             n = nb_sectors;
-        cluster_offset = get_cluster_offset(bs, sector_num << 9, 1, 0, 
+        not_zero = is_not_zero(buf, n * 512);
+        cluster_offset = get_cluster_offset(bs, sector_num << 9, not_zero, 0, 
                                             index_in_cluster, 
                                             index_in_cluster + n);
-        if (!cluster_offset)
-            return -1;
+        if (!cluster_offset) {
+            if (not_zero)
+                return -1;
+            goto next_cluster;
+        }
         if (s->crypt_method) {
             encrypt_sectors(s, sector_num, s->cluster_data, buf, n, 1,
                             &s->aes_encrypt_key);
             ret = bdrv_pwrite(s->hd, cluster_offset + index_in_cluster * 512, 
                               s->cluster_data, n * 512);
         } else {
             ret = bdrv_pwrite(s->hd, cluster_offset + index_in_cluster * 512, buf, n * 512);
         }
         if (ret != n * 512) 
             return -1;
+next_cluster:
         nb_sectors -= n;
         sector_num += n;
         buf += n * 512;
     }
     s->cluster_cache_offset = -1; /* disable compressed cache */
     return 0;
 }
 
 typedef struct {
     int64_t sector_num;

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2006-08-02 17:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-07-26  8:23 [Qemu-devel] Wipe patch ZIGLIO, Frediano, VF-IT
2006-07-26  8:41 ` Brad Campbell
2006-07-26  9:19   ` Avi Kivity
2006-07-26  9:35     ` Brad Campbell
2006-08-02  2:25 ` Brad Campbell
2006-08-02  2:49   ` andrzej zaborowski
2006-08-02  2:51     ` andrzej zaborowski
2006-08-02  3:09       ` Brad Campbell
2006-08-02  7:39     ` Nigel Horne
2006-08-02 17:32   ` Andreas Bollhalder
  -- strict thread matches above, loose matches on Subject: below --
2006-07-26  8:48 ZIGLIO, Frediano, VF-IT
2006-07-26  9:42 ZIGLIO, Frediano, VF-IT
2006-07-26  9:52 ` Brad Campbell
2006-08-02 12:53 ZIGLIO, Frediano, VF-IT

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).