From mboxrd@z Thu Jan 1 00:00:00 1970 From: Asias He Subject: Re: [PATCH] kvm tools: QCOW code cleanups Date: Thu, 12 May 2011 08:56:39 +0800 Message-ID: <4DCB3047.7030300@gmail.com> References: <1305136142-14018-1-git-send-email-penberg@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, Avi Kivity , Cyrill Gorcunov , Ingo Molnar , Prasad Joshi , Sasha Levin To: Pekka Enberg Return-path: Received: from mail-pz0-f46.google.com ([209.85.210.46]:38463 "EHLO mail-pz0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750940Ab1ELA55 (ORCPT ); Wed, 11 May 2011 20:57:57 -0400 Received: by pzk9 with SMTP id 9so487351pzk.19 for ; Wed, 11 May 2011 17:57:56 -0700 (PDT) In-Reply-To: <1305136142-14018-1-git-send-email-penberg@kernel.org> Sender: kvm-owner@vger.kernel.org List-ID: On 05/12/2011 01:49 AM, Pekka Enberg wrote: > Fix up coding style issues in QCOW code. >=20 > Cc: Asias He > Cc: Avi Kivity > Cc: Cyrill Gorcunov > Cc: Ingo Molnar > Cc: Prasad Joshi > Cc: Sasha Levin > Signed-off-by: Pekka Enberg > --- > tools/kvm/qcow.c | 153 ++++++++++++++++++++++++++++----------------= ---------- > 1 files changed, 80 insertions(+), 73 deletions(-) >=20 > 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 > =20 > #include > -#include > #include > +#include > =20 > 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 =3D q->header; > struct qcow_table *table =3D &q->table; > - u64 *l2_table =3D 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; > =20 > + l2_table =3D NULL; > + > cluster_size =3D 1 << header->cluster_bits; > =20 > l1_idx =3D get_l1_index(q, offset); > @@ -74,8 +76,7 @@ static ssize_t qcow1_read_cluster(struct qcow *q, u= 64 offset, void *dst, u32 dst > if (!l2_table) > goto out_error; > =20 > - 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; > =20 > l2_idx =3D get_l2_index(q, offset); > @@ -102,80 +103,82 @@ out_error: > goto out; > } > =20 > -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, vo= id *dst, u32 dst_len) > { > struct qcow *q =3D disk->priv; > struct qcow_header *header =3D q->header; > - char *buf =3D dst; > - u64 offset; > u32 nr_read; > + u64 offset; > + char *buf; > u32 nr; > =20 > - nr_read =3D 0; > + buf =3D dst; > + nr_read =3D 0; > + > while (nr_read < dst_len) { > - offset =3D sector << SECTOR_SHIFT; > + offset =3D sector << SECTOR_SHIFT; > if (offset >=3D header->size) > - goto out_error; > + return -1; > =20 > nr =3D qcow1_read_cluster(q, offset, buf, dst_len - nr_read); > if (nr <=3D 0) > - goto out_error; > + return -1; > =20 > nr_read +=3D nr; > buf +=3D nr; > sector +=3D (nr >> SECTOR_SHIFT); > } > + > return 0; > -out_error: > - return -1; > } > =20 > static inline u64 file_size(int fd) > { > struct stat st; > + > if (fstat(fd, &st) < 0) > return 0; > + > return st.st_size; > } > =20 > -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_W= RITE) > + > +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); > } > =20 > /* 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 =3D q->header; > - u64 sz; > u64 clust_sz; > - u64 off; > u64 f_sz; > + u64 off; > + u64 sz; > =20 > - f_sz =3D file_size(q->fd); > + f_sz =3D file_size(q->fd); > if (!f_sz) > return 0; > =20 > - sz =3D 1 << header->l2_bits; > - clust_sz =3D 1 << header->cluster_bits; > - off =3D ALIGN(f_sz, clust_sz); > + sz =3D 1 << header->l2_bits; > + clust_sz =3D 1 << header->cluster_bits; > + off =3D ALIGN(f_sz, clust_sz); > =20 > - 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; > } > =20 > /* > * 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_s= ync() to > * synchronize the in-core state of QCOW image to disk. > * > * We also try to restore the image to a consistent state if the met= data > @@ -186,36 +189,35 @@ static ssize_t qcow1_write_cluster(struct qcow = *q, u64 offset, void *buf, u32 sr > { > struct qcow_header *header =3D q->header; > struct qcow_table *table =3D &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 =3D false; > =20 > - l2t_sz =3D 1 << header->l2_bits; > - clust_sz =3D 1 << header->cluster_bits; > + l2t_sz =3D 1 << header->l2_bits; > + clust_sz =3D 1 << header->cluster_bits; > =20 > - l1t_idx =3D get_l1_index(q, offset); > + l1t_idx =3D get_l1_index(q, offset); > if (l1t_idx >=3D table->table_size) > goto error; > =20 > - l2t_idx =3D get_l2_index(q, offset); > + l2t_idx =3D get_l2_index(q, offset); > if (l2t_idx >=3D l2t_sz) > goto error; > =20 > - clust_off =3D get_cluster_offset(q, offset); > + clust_off =3D get_cluster_offset(q, offset); > if (clust_off >=3D clust_sz) > goto error; > =20 > - len =3D clust_sz - clust_off; > + len =3D clust_sz - clust_off; > if (len > src_len) > len =3D src_len; > =20 > @@ -223,61 +225,65 @@ static ssize_t qcow1_write_cluster(struct qcow = *q, u64 offset, void *buf, u32 sr > if (!l2t) > goto error; > =20 > - l2t_off =3D table->l1_table[l1t_idx] & ~header->oflag_mask; > + l2t_off =3D 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 =3D file_size(q->fd); > + /* Capture the state of the consistent QCOW image */ > + f_sz =3D file_size(q->fd); > if (!f_sz) > goto free_l2; > =20 > /* Write the l2 table of 0's at the end of the file */ > - l2t_off =3D qcow1_write_l2_table(q, l2t); > + l2t_off =3D qcow1_write_l2_table(q, l2t); > if (!l2t_off) > goto free_l2; > =20 > /* Metadata update: update on disk level 1 table */ > - t =3D cpu_to_be64(l2t_off); > - if (pwrite_sync(q->fd, &t, sizeof(t), header->l1_table_offset + > - l1t_idx * sizeof(u64)) < 0) { > + t =3D 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; > } > =20 > - /* update the in-core entry */ > + /* Update the in-core entry */ > table->l1_table[l1t_idx] =3D l2t_off; > } > =20 > - /* capture the state of the consistent QCOW image */ > - f_sz =3D file_size(q->fd); > + /* Capture the state of the consistent QCOW image */ > + f_sz =3D file_size(q->fd); > if (!f_sz) > goto free_l2; > =20 > - clust_start =3D be64_to_cpu(l2t[l2t_idx]) & ~header->oflag_mask; > - free(l2t); > + clust_start =3D be64_to_cpu(l2t[l2t_idx]) & ~header->oflag_mask; > if (!clust_start) { > - clust_start =3D ALIGN(f_sz, clust_sz); > - update_meta =3D true; > - } > + clust_start =3D ALIGN(f_sz, clust_sz); > + update_meta =3D true; > + } else > + update_meta =3D false; > =20 > - /* write actual data */ > + free(l2t); > + > + /* Write actual data */ > if (pwrite_in_full(q->fd, buf, len, clust_start + clust_off) < 0) > goto error; > =20 > if (update_meta) { > t =3D 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 * siz= eof(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 =3D disk->priv; > struct qcow_header *header =3D q->header; > - char *buf =3D src; > - ssize_t nr_write; > + ssize_t nr_written; > + char *buf; > u64 offset; > ssize_t nr; > =20 > - nr_write =3D 0; > - offset =3D sector << SECTOR_SHIFT; > - while (nr_write < src_len) { > + buf =3D src; > + nr_written =3D 0; > + offset =3D sector << SECTOR_SHIFT; > + > + while (nr_written < src_len) { The above line gives me: CC qcow.o qcow.c: In function =E2=80=98qcow1_write_sector=E2=80=99: qcow.c:307:20: error: comparison between signed and unsigned integer expressions [-Werror=3Dsign-compare] cc1: all warnings being treated as errors make: *** [qcow.o] Error 1 > if (offset >=3D header->size) > - goto error; > + return -1; > =20 > - nr =3D qcow1_write_cluster(q, offset, buf, src_len - nr_write); > + nr =3D qcow1_write_cluster(q, offset, buf, src_len - nr_written); > if (nr < 0) > - goto error; > + return -1; > =20 > - nr_write +=3D nr; > - buf +=3D nr; > - offset +=3D nr; > + nr_written +=3D nr; > + buf +=3D nr; > + offset +=3D nr; > } > + > return 0; > -error: > - return -1; > } > =20 > static void qcow1_disk_close(struct disk_image *disk) --=20 Best Regards, Asias He