* [PATCH] kvm tools: QCOW code cleanups
@ 2011-05-11 17:49 Pekka Enberg
2011-05-12 0:56 ` Asias He
0 siblings, 1 reply; 2+ messages in thread
From: Pekka Enberg @ 2011-05-11 17:49 UTC (permalink / raw)
To: kvm
Cc: Pekka Enberg, Asias He, Avi Kivity, Cyrill Gorcunov, Ingo Molnar,
Prasad Joshi, Sasha Levin
Fix up coding style issues in QCOW code.
Cc: Asias He <asias.hejun@gmail.com>
Cc: Avi Kivity <avi@redhat.com>
Cc: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Prasad Joshi <prasadjoshi124@gmail.com>
Cc: Sasha Levin <levinsasha928@gmail.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
---
tools/kvm/qcow.c | 153 ++++++++++++++++++++++++++++--------------------------
1 files changed, 80 insertions(+), 73 deletions(-)
diff --git a/tools/kvm/qcow.c b/tools/kvm/qcow.c
index 103a5a1..f8b52d4 100644
--- a/tools/kvm/qcow.c
+++ b/tools/kvm/qcow.c
@@ -13,8 +13,8 @@
#include <fcntl.h>
#include <linux/byteorder.h>
-#include <linux/types.h>
#include <linux/kernel.h>
+#include <linux/types.h>
static inline u64 get_l1_index(struct qcow *q, u64 offset)
{
@@ -41,16 +41,18 @@ static ssize_t qcow1_read_cluster(struct qcow *q, u64 offset, void *dst, u32 dst
{
struct qcow_header *header = q->header;
struct qcow_table *table = &q->table;
- u64 *l2_table = NULL;
u64 l2_table_offset;
u64 l2_table_size;
u64 cluster_size;
u64 clust_offset;
u64 clust_start;
size_t length;
+ u64 *l2_table;
u64 l1_idx;
u64 l2_idx;
+ l2_table = NULL;
+
cluster_size = 1 << header->cluster_bits;
l1_idx = get_l1_index(q, offset);
@@ -74,8 +76,7 @@ static ssize_t qcow1_read_cluster(struct qcow *q, u64 offset, void *dst, u32 dst
if (!l2_table)
goto out_error;
- if (pread_in_full(q->fd, l2_table, l2_table_size * sizeof(u64),
- l2_table_offset) < 0)
+ if (pread_in_full(q->fd, l2_table, l2_table_size * sizeof(u64), l2_table_offset) < 0)
goto out_error;
l2_idx = get_l2_index(q, offset);
@@ -102,80 +103,82 @@ out_error:
goto out;
}
-static int qcow1_read_sector(struct disk_image *disk, u64 sector,
- void *dst, u32 dst_len)
+static int qcow1_read_sector(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
{
struct qcow *q = disk->priv;
struct qcow_header *header = q->header;
- char *buf = dst;
- u64 offset;
u32 nr_read;
+ u64 offset;
+ char *buf;
u32 nr;
- nr_read = 0;
+ buf = dst;
+ nr_read = 0;
+
while (nr_read < dst_len) {
- offset = sector << SECTOR_SHIFT;
+ offset = sector << SECTOR_SHIFT;
if (offset >= header->size)
- goto out_error;
+ return -1;
nr = qcow1_read_cluster(q, offset, buf, dst_len - nr_read);
if (nr <= 0)
- goto out_error;
+ return -1;
nr_read += nr;
buf += nr;
sector += (nr >> SECTOR_SHIFT);
}
+
return 0;
-out_error:
- return -1;
}
static inline u64 file_size(int fd)
{
struct stat st;
+
if (fstat(fd, &st) < 0)
return 0;
+
return st.st_size;
}
-static inline int pwrite_sync(int fd, void *buf, size_t count, off_t offset)
+#define SYNC_FLAGS (SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE)
+
+static inline int qcow_pwrite_sync(int fd, void *buf, size_t count, off_t offset)
{
if (pwrite_in_full(fd, buf, count, offset) < 0)
return -1;
- if (sync_file_range(fd, offset, count,
- SYNC_FILE_RANGE_WAIT_BEFORE |
- SYNC_FILE_RANGE_WRITE) < 0)
- return -1;
- return 0;
+
+ return sync_file_range(fd, offset, count, SYNC_FLAGS);
}
/* Writes a level 2 table at the end of the file. */
static u64 qcow1_write_l2_table(struct qcow *q, u64 *table)
{
struct qcow_header *header = q->header;
- u64 sz;
u64 clust_sz;
- u64 off;
u64 f_sz;
+ u64 off;
+ u64 sz;
- f_sz = file_size(q->fd);
+ f_sz = file_size(q->fd);
if (!f_sz)
return 0;
- sz = 1 << header->l2_bits;
- clust_sz = 1 << header->cluster_bits;
- off = ALIGN(f_sz, clust_sz);
+ sz = 1 << header->l2_bits;
+ clust_sz = 1 << header->cluster_bits;
+ off = ALIGN(f_sz, clust_sz);
- if (pwrite_sync(q->fd, table, sz * sizeof(u64), off) < 0)
+ if (qcow_pwrite_sync(q->fd, table, sz * sizeof(u64), off) < 0)
return 0;
+
return off;
}
/*
* QCOW file might grow during a write operation. Not only data but metadata is
* also written at the end of the file. Therefore it is necessary to ensure
- * every write is committed to disk. Hence we use uses pwrite_sync() to
+ * every write is committed to disk. Hence we use uses qcow_pwrite_sync() to
* synchronize the in-core state of QCOW image to disk.
*
* We also try to restore the image to a consistent state if the metdata
@@ -186,36 +189,35 @@ static ssize_t qcow1_write_cluster(struct qcow *q, u64 offset, void *buf, u32 sr
{
struct qcow_header *header = q->header;
struct qcow_table *table = &q->table;
-
- u64 l2t_sz;
+ bool update_meta;
+ u64 clust_start;
+ u64 clust_off;
u64 clust_sz;
u64 l1t_idx;
u64 l2t_idx;
- u64 clust_off;
- u64 len;
+ u64 l2t_off;
+ u64 l2t_sz;
u64 *l2t;
u64 f_sz;
- u64 l2t_off;
+ u64 len;
u64 t;
- u64 clust_start;
- bool update_meta = false;
- l2t_sz = 1 << header->l2_bits;
- clust_sz = 1 << header->cluster_bits;
+ l2t_sz = 1 << header->l2_bits;
+ clust_sz = 1 << header->cluster_bits;
- l1t_idx = get_l1_index(q, offset);
+ l1t_idx = get_l1_index(q, offset);
if (l1t_idx >= table->table_size)
goto error;
- l2t_idx = get_l2_index(q, offset);
+ l2t_idx = get_l2_index(q, offset);
if (l2t_idx >= l2t_sz)
goto error;
- clust_off = get_cluster_offset(q, offset);
+ clust_off = get_cluster_offset(q, offset);
if (clust_off >= clust_sz)
goto error;
- len = clust_sz - clust_off;
+ len = clust_sz - clust_off;
if (len > src_len)
len = src_len;
@@ -223,61 +225,65 @@ static ssize_t qcow1_write_cluster(struct qcow *q, u64 offset, void *buf, u32 sr
if (!l2t)
goto error;
- l2t_off = table->l1_table[l1t_idx] & ~header->oflag_mask;
+ l2t_off = table->l1_table[l1t_idx] & ~header->oflag_mask;
if (l2t_off) {
if (pread_in_full(q->fd, l2t, l2t_sz * sizeof(u64), l2t_off) < 0)
goto free_l2;
} else {
- /* capture the state of the consistent QCOW image */
- f_sz = file_size(q->fd);
+ /* Capture the state of the consistent QCOW image */
+ f_sz = file_size(q->fd);
if (!f_sz)
goto free_l2;
/* Write the l2 table of 0's at the end of the file */
- l2t_off = qcow1_write_l2_table(q, l2t);
+ l2t_off = qcow1_write_l2_table(q, l2t);
if (!l2t_off)
goto free_l2;
/* Metadata update: update on disk level 1 table */
- t = cpu_to_be64(l2t_off);
- if (pwrite_sync(q->fd, &t, sizeof(t), header->l1_table_offset +
- l1t_idx * sizeof(u64)) < 0) {
+ t = cpu_to_be64(l2t_off);
+
+ if (qcow_pwrite_sync(q->fd, &t, sizeof(t), header->l1_table_offset + l1t_idx * sizeof(u64)) < 0) {
/* restore file to consistent state */
if (ftruncate(q->fd, f_sz) < 0)
goto free_l2;
+
goto free_l2;
}
- /* update the in-core entry */
+ /* Update the in-core entry */
table->l1_table[l1t_idx] = l2t_off;
}
- /* capture the state of the consistent QCOW image */
- f_sz = file_size(q->fd);
+ /* Capture the state of the consistent QCOW image */
+ f_sz = file_size(q->fd);
if (!f_sz)
goto free_l2;
- clust_start = be64_to_cpu(l2t[l2t_idx]) & ~header->oflag_mask;
- free(l2t);
+ clust_start = be64_to_cpu(l2t[l2t_idx]) & ~header->oflag_mask;
if (!clust_start) {
- clust_start = ALIGN(f_sz, clust_sz);
- update_meta = true;
- }
+ clust_start = ALIGN(f_sz, clust_sz);
+ update_meta = true;
+ } else
+ update_meta = false;
- /* write actual data */
+ free(l2t);
+
+ /* Write actual data */
if (pwrite_in_full(q->fd, buf, len, clust_start + clust_off) < 0)
goto error;
if (update_meta) {
t = cpu_to_be64(clust_start);
- if (pwrite_sync(q->fd, &t, sizeof(t), l2t_off +
- l2t_idx * sizeof(u64)) < 0) {
- /* restore the file to consistent state */
+ if (qcow_pwrite_sync(q->fd, &t, sizeof(t), l2t_off + l2t_idx * sizeof(u64)) < 0) {
+ /* Restore the file to consistent state */
if (ftruncate(q->fd, f_sz) < 0)
goto error;
+
goto error;
}
}
+
return len;
free_l2:
free(l2t);
@@ -289,28 +295,29 @@ static int qcow1_write_sector(struct disk_image *disk, u64 sector, void *src, u3
{
struct qcow *q = disk->priv;
struct qcow_header *header = q->header;
- char *buf = src;
- ssize_t nr_write;
+ ssize_t nr_written;
+ char *buf;
u64 offset;
ssize_t nr;
- nr_write = 0;
- offset = sector << SECTOR_SHIFT;
- while (nr_write < src_len) {
+ buf = src;
+ nr_written = 0;
+ offset = sector << SECTOR_SHIFT;
+
+ while (nr_written < src_len) {
if (offset >= header->size)
- goto error;
+ return -1;
- nr = qcow1_write_cluster(q, offset, buf, src_len - nr_write);
+ nr = qcow1_write_cluster(q, offset, buf, src_len - nr_written);
if (nr < 0)
- goto error;
+ return -1;
- nr_write += nr;
- buf += nr;
- offset += nr;
+ nr_written += nr;
+ buf += nr;
+ offset += nr;
}
+
return 0;
-error:
- return -1;
}
static void qcow1_disk_close(struct disk_image *disk)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] kvm tools: QCOW code cleanups
2011-05-11 17:49 [PATCH] kvm tools: QCOW code cleanups Pekka Enberg
@ 2011-05-12 0:56 ` Asias He
0 siblings, 0 replies; 2+ messages in thread
From: Asias He @ 2011-05-12 0:56 UTC (permalink / raw)
To: Pekka Enberg
Cc: kvm, Avi Kivity, Cyrill Gorcunov, Ingo Molnar, Prasad Joshi,
Sasha Levin
On 05/12/2011 01:49 AM, Pekka Enberg wrote:
> Fix up coding style issues in QCOW code.
>
> Cc: Asias He <asias.hejun@gmail.com>
> Cc: Avi Kivity <avi@redhat.com>
> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Prasad Joshi <prasadjoshi124@gmail.com>
> Cc: Sasha Levin <levinsasha928@gmail.com>
> Signed-off-by: Pekka Enberg <penberg@kernel.org>
> ---
> tools/kvm/qcow.c | 153 ++++++++++++++++++++++++++++--------------------------
> 1 files changed, 80 insertions(+), 73 deletions(-)
>
> diff --git a/tools/kvm/qcow.c b/tools/kvm/qcow.c
> index 103a5a1..f8b52d4 100644
> --- a/tools/kvm/qcow.c
> +++ b/tools/kvm/qcow.c
> @@ -13,8 +13,8 @@
> #include <fcntl.h>
>
> #include <linux/byteorder.h>
> -#include <linux/types.h>
> #include <linux/kernel.h>
> +#include <linux/types.h>
>
> static inline u64 get_l1_index(struct qcow *q, u64 offset)
> {
> @@ -41,16 +41,18 @@ static ssize_t qcow1_read_cluster(struct qcow *q, u64 offset, void *dst, u32 dst
> {
> struct qcow_header *header = q->header;
> struct qcow_table *table = &q->table;
> - u64 *l2_table = NULL;
> u64 l2_table_offset;
> u64 l2_table_size;
> u64 cluster_size;
> u64 clust_offset;
> u64 clust_start;
> size_t length;
> + u64 *l2_table;
> u64 l1_idx;
> u64 l2_idx;
>
> + l2_table = NULL;
> +
> cluster_size = 1 << header->cluster_bits;
>
> l1_idx = get_l1_index(q, offset);
> @@ -74,8 +76,7 @@ static ssize_t qcow1_read_cluster(struct qcow *q, u64 offset, void *dst, u32 dst
> if (!l2_table)
> goto out_error;
>
> - if (pread_in_full(q->fd, l2_table, l2_table_size * sizeof(u64),
> - l2_table_offset) < 0)
> + if (pread_in_full(q->fd, l2_table, l2_table_size * sizeof(u64), l2_table_offset) < 0)
> goto out_error;
>
> l2_idx = get_l2_index(q, offset);
> @@ -102,80 +103,82 @@ out_error:
> goto out;
> }
>
> -static int qcow1_read_sector(struct disk_image *disk, u64 sector,
> - void *dst, u32 dst_len)
> +static int qcow1_read_sector(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
> {
> struct qcow *q = disk->priv;
> struct qcow_header *header = q->header;
> - char *buf = dst;
> - u64 offset;
> u32 nr_read;
> + u64 offset;
> + char *buf;
> u32 nr;
>
> - nr_read = 0;
> + buf = dst;
> + nr_read = 0;
> +
> while (nr_read < dst_len) {
> - offset = sector << SECTOR_SHIFT;
> + offset = sector << SECTOR_SHIFT;
> if (offset >= header->size)
> - goto out_error;
> + return -1;
>
> nr = qcow1_read_cluster(q, offset, buf, dst_len - nr_read);
> if (nr <= 0)
> - goto out_error;
> + return -1;
>
> nr_read += nr;
> buf += nr;
> sector += (nr >> SECTOR_SHIFT);
> }
> +
> return 0;
> -out_error:
> - return -1;
> }
>
> static inline u64 file_size(int fd)
> {
> struct stat st;
> +
> if (fstat(fd, &st) < 0)
> return 0;
> +
> return st.st_size;
> }
>
> -static inline int pwrite_sync(int fd, void *buf, size_t count, off_t offset)
> +#define SYNC_FLAGS (SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE)
> +
> +static inline int qcow_pwrite_sync(int fd, void *buf, size_t count, off_t offset)
> {
> if (pwrite_in_full(fd, buf, count, offset) < 0)
> return -1;
> - if (sync_file_range(fd, offset, count,
> - SYNC_FILE_RANGE_WAIT_BEFORE |
> - SYNC_FILE_RANGE_WRITE) < 0)
> - return -1;
> - return 0;
> +
> + return sync_file_range(fd, offset, count, SYNC_FLAGS);
> }
>
> /* Writes a level 2 table at the end of the file. */
> static u64 qcow1_write_l2_table(struct qcow *q, u64 *table)
> {
> struct qcow_header *header = q->header;
> - u64 sz;
> u64 clust_sz;
> - u64 off;
> u64 f_sz;
> + u64 off;
> + u64 sz;
>
> - f_sz = file_size(q->fd);
> + f_sz = file_size(q->fd);
> if (!f_sz)
> return 0;
>
> - sz = 1 << header->l2_bits;
> - clust_sz = 1 << header->cluster_bits;
> - off = ALIGN(f_sz, clust_sz);
> + sz = 1 << header->l2_bits;
> + clust_sz = 1 << header->cluster_bits;
> + off = ALIGN(f_sz, clust_sz);
>
> - if (pwrite_sync(q->fd, table, sz * sizeof(u64), off) < 0)
> + if (qcow_pwrite_sync(q->fd, table, sz * sizeof(u64), off) < 0)
> return 0;
> +
> return off;
> }
>
> /*
> * QCOW file might grow during a write operation. Not only data but metadata is
> * also written at the end of the file. Therefore it is necessary to ensure
> - * every write is committed to disk. Hence we use uses pwrite_sync() to
> + * every write is committed to disk. Hence we use uses qcow_pwrite_sync() to
> * synchronize the in-core state of QCOW image to disk.
> *
> * We also try to restore the image to a consistent state if the metdata
> @@ -186,36 +189,35 @@ static ssize_t qcow1_write_cluster(struct qcow *q, u64 offset, void *buf, u32 sr
> {
> struct qcow_header *header = q->header;
> struct qcow_table *table = &q->table;
> -
> - u64 l2t_sz;
> + bool update_meta;
> + u64 clust_start;
> + u64 clust_off;
> u64 clust_sz;
> u64 l1t_idx;
> u64 l2t_idx;
> - u64 clust_off;
> - u64 len;
> + u64 l2t_off;
> + u64 l2t_sz;
> u64 *l2t;
> u64 f_sz;
> - u64 l2t_off;
> + u64 len;
> u64 t;
> - u64 clust_start;
> - bool update_meta = false;
>
> - l2t_sz = 1 << header->l2_bits;
> - clust_sz = 1 << header->cluster_bits;
> + l2t_sz = 1 << header->l2_bits;
> + clust_sz = 1 << header->cluster_bits;
>
> - l1t_idx = get_l1_index(q, offset);
> + l1t_idx = get_l1_index(q, offset);
> if (l1t_idx >= table->table_size)
> goto error;
>
> - l2t_idx = get_l2_index(q, offset);
> + l2t_idx = get_l2_index(q, offset);
> if (l2t_idx >= l2t_sz)
> goto error;
>
> - clust_off = get_cluster_offset(q, offset);
> + clust_off = get_cluster_offset(q, offset);
> if (clust_off >= clust_sz)
> goto error;
>
> - len = clust_sz - clust_off;
> + len = clust_sz - clust_off;
> if (len > src_len)
> len = src_len;
>
> @@ -223,61 +225,65 @@ static ssize_t qcow1_write_cluster(struct qcow *q, u64 offset, void *buf, u32 sr
> if (!l2t)
> goto error;
>
> - l2t_off = table->l1_table[l1t_idx] & ~header->oflag_mask;
> + l2t_off = table->l1_table[l1t_idx] & ~header->oflag_mask;
> if (l2t_off) {
> if (pread_in_full(q->fd, l2t, l2t_sz * sizeof(u64), l2t_off) < 0)
> goto free_l2;
> } else {
> - /* capture the state of the consistent QCOW image */
> - f_sz = file_size(q->fd);
> + /* Capture the state of the consistent QCOW image */
> + f_sz = file_size(q->fd);
> if (!f_sz)
> goto free_l2;
>
> /* Write the l2 table of 0's at the end of the file */
> - l2t_off = qcow1_write_l2_table(q, l2t);
> + l2t_off = qcow1_write_l2_table(q, l2t);
> if (!l2t_off)
> goto free_l2;
>
> /* Metadata update: update on disk level 1 table */
> - t = cpu_to_be64(l2t_off);
> - if (pwrite_sync(q->fd, &t, sizeof(t), header->l1_table_offset +
> - l1t_idx * sizeof(u64)) < 0) {
> + t = cpu_to_be64(l2t_off);
> +
> + if (qcow_pwrite_sync(q->fd, &t, sizeof(t), header->l1_table_offset + l1t_idx * sizeof(u64)) < 0) {
> /* restore file to consistent state */
> if (ftruncate(q->fd, f_sz) < 0)
> goto free_l2;
> +
> goto free_l2;
> }
>
> - /* update the in-core entry */
> + /* Update the in-core entry */
> table->l1_table[l1t_idx] = l2t_off;
> }
>
> - /* capture the state of the consistent QCOW image */
> - f_sz = file_size(q->fd);
> + /* Capture the state of the consistent QCOW image */
> + f_sz = file_size(q->fd);
> if (!f_sz)
> goto free_l2;
>
> - clust_start = be64_to_cpu(l2t[l2t_idx]) & ~header->oflag_mask;
> - free(l2t);
> + clust_start = be64_to_cpu(l2t[l2t_idx]) & ~header->oflag_mask;
> if (!clust_start) {
> - clust_start = ALIGN(f_sz, clust_sz);
> - update_meta = true;
> - }
> + clust_start = ALIGN(f_sz, clust_sz);
> + update_meta = true;
> + } else
> + update_meta = false;
>
> - /* write actual data */
> + free(l2t);
> +
> + /* Write actual data */
> if (pwrite_in_full(q->fd, buf, len, clust_start + clust_off) < 0)
> goto error;
>
> if (update_meta) {
> t = cpu_to_be64(clust_start);
> - if (pwrite_sync(q->fd, &t, sizeof(t), l2t_off +
> - l2t_idx * sizeof(u64)) < 0) {
> - /* restore the file to consistent state */
> + if (qcow_pwrite_sync(q->fd, &t, sizeof(t), l2t_off + l2t_idx * sizeof(u64)) < 0) {
> + /* Restore the file to consistent state */
> if (ftruncate(q->fd, f_sz) < 0)
> goto error;
> +
> goto error;
> }
> }
> +
> return len;
> free_l2:
> free(l2t);
> @@ -289,28 +295,29 @@ static int qcow1_write_sector(struct disk_image *disk, u64 sector, void *src, u3
> {
> struct qcow *q = disk->priv;
> struct qcow_header *header = q->header;
> - char *buf = src;
> - ssize_t nr_write;
> + ssize_t nr_written;
> + char *buf;
> u64 offset;
> ssize_t nr;
>
> - nr_write = 0;
> - offset = sector << SECTOR_SHIFT;
> - while (nr_write < src_len) {
> + buf = src;
> + nr_written = 0;
> + offset = sector << SECTOR_SHIFT;
> +
> + while (nr_written < src_len) {
The above line gives me:
CC qcow.o
qcow.c: In function ‘qcow1_write_sector’:
qcow.c:307:20: error: comparison between signed and unsigned integer
expressions [-Werror=sign-compare]
cc1: all warnings being treated as errors
make: *** [qcow.o] Error 1
> if (offset >= header->size)
> - goto error;
> + return -1;
>
> - nr = qcow1_write_cluster(q, offset, buf, src_len - nr_write);
> + nr = qcow1_write_cluster(q, offset, buf, src_len - nr_written);
> if (nr < 0)
> - goto error;
> + return -1;
>
> - nr_write += nr;
> - buf += nr;
> - offset += nr;
> + nr_written += nr;
> + buf += nr;
> + offset += nr;
> }
> +
> return 0;
> -error:
> - return -1;
> }
>
> static void qcow1_disk_close(struct disk_image *disk)
--
Best Regards,
Asias He
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-05-12 0:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-11 17:49 [PATCH] kvm tools: QCOW code cleanups Pekka Enberg
2011-05-12 0:56 ` Asias He
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox