* [Qemu-devel] [PATCH 0/2] qcow2 cleanups @ 2010-12-16 16:05 Jes.Sorensen 2010-12-16 16:05 ` [Qemu-devel] [PATCH 1/2] block/qcow2.c: rename qcow_ functions to qcow2_ Jes.Sorensen 2010-12-16 16:05 ` [Qemu-devel] [PATCH 2/2] Add proper -errno error return values to qcow2_open() Jes.Sorensen 0 siblings, 2 replies; 8+ messages in thread From: Jes.Sorensen @ 2010-12-16 16:05 UTC (permalink / raw) To: kwolf; +Cc: qemu-devel From: Jes Sorensen <Jes.Sorensen@redhat.com> Hi, These two patches tries to clean up the qcow2 code a little. First makes the function names consistent, ie. we shouldn't have qcow_ functions in the qcow2 code. Second tries to add proper errno return values to qcow2_open(). Jes Sorensen (2): block/qcow2.c: rename qcow_ functions to qcow2_ Add proper -errno error return values to qcow2_open() block/qcow2-cluster.c | 6 +- block/qcow2-snapshot.c | 6 +- block/qcow2.c | 269 +++++++++++++++++++++++++++--------------------- 3 files changed, 158 insertions(+), 123 deletions(-) -- 1.7.3.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/2] block/qcow2.c: rename qcow_ functions to qcow2_ 2010-12-16 16:05 [Qemu-devel] [PATCH 0/2] qcow2 cleanups Jes.Sorensen @ 2010-12-16 16:05 ` Jes.Sorensen 2010-12-17 14:20 ` [Qemu-devel] " Kevin Wolf 2010-12-16 16:05 ` [Qemu-devel] [PATCH 2/2] Add proper -errno error return values to qcow2_open() Jes.Sorensen 1 sibling, 1 reply; 8+ messages in thread From: Jes.Sorensen @ 2010-12-16 16:05 UTC (permalink / raw) To: kwolf; +Cc: qemu-devel From: Jes Sorensen <Jes.Sorensen@redhat.com> It doesn't really make sense for functions in qcow2.c to be named qcow_ so convert the names to match correctly. Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> --- block/qcow2-cluster.c | 6 +- block/qcow2-snapshot.c | 6 +- block/qcow2.c | 210 +++++++++++++++++++++++++----------------------- 3 files changed, 116 insertions(+), 106 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index b040208..6928c63 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -352,8 +352,8 @@ void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, } -static int qcow_read(BlockDriverState *bs, int64_t sector_num, - uint8_t *buf, int nb_sectors) +static int qcow2_read(BlockDriverState *bs, int64_t sector_num, + uint8_t *buf, int nb_sectors) { BDRVQcowState *s = bs->opaque; int ret, index_in_cluster, n, n1; @@ -419,7 +419,7 @@ static int copy_sectors(BlockDriverState *bs, uint64_t start_sect, if (n <= 0) return 0; BLKDBG_EVENT(bs->file, BLKDBG_COW_READ); - ret = qcow_read(bs, start_sect + n_start, s->cluster_data, n); + ret = qcow2_read(bs, start_sect + n_start, s->cluster_data, n); if (ret < 0) return ret; if (s->crypt_method) { diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index aacf357..74823a5 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -116,7 +116,7 @@ int qcow2_read_snapshots(BlockDriverState *bs) } /* add at the end of the file a new list of snapshots */ -static int qcow_write_snapshots(BlockDriverState *bs) +static int qcow2_write_snapshots(BlockDriverState *bs) { BDRVQcowState *s = bs->opaque; QCowSnapshot *sn; @@ -300,7 +300,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) s->snapshots = snapshots1; s->snapshots[s->nb_snapshots++] = *sn; - if (qcow_write_snapshots(bs) < 0) + if (qcow2_write_snapshots(bs) < 0) goto fail; #ifdef DEBUG_ALLOC qcow2_check_refcounts(bs); @@ -378,7 +378,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id) qemu_free(sn->name); memmove(sn, sn + 1, (s->nb_snapshots - snapshot_index - 1) * sizeof(*sn)); s->nb_snapshots--; - ret = qcow_write_snapshots(bs); + ret = qcow2_write_snapshots(bs); if (ret < 0) { /* XXX: restore snapshot if error ? */ return ret; diff --git a/block/qcow2.c b/block/qcow2.c index 537c479..d7fd167 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -50,10 +50,10 @@ typedef struct { uint32_t magic; uint32_t len; } QCowExtension; -#define QCOW_EXT_MAGIC_END 0 -#define QCOW_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA +#define QCOW2_EXT_MAGIC_END 0 +#define QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA -static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename) +static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename) { const QCowHeader *cow_header = (const void *)buf; @@ -73,14 +73,14 @@ static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename) * unknown magic is skipped (future extension this version knows nothing about) * return 0 upon success, non-0 otherwise */ -static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset, - uint64_t end_offset) +static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, + uint64_t end_offset) { QCowExtension ext; uint64_t offset; #ifdef DEBUG_EXT - printf("qcow_read_extensions: start=%ld end=%ld\n", start_offset, end_offset); + printf("qcow2_read_extensions: start=%ld end=%ld\n", start_offset, end_offset); #endif offset = start_offset; while (offset < end_offset) { @@ -88,13 +88,13 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset, #ifdef DEBUG_EXT /* Sanity check */ if (offset > s->cluster_size) - printf("qcow_handle_extension: suspicious offset %lu\n", offset); + printf("qcow_read_extension: suspicious offset %lu\n", offset); printf("attemting to read extended header in offset %lu\n", offset); #endif if (bdrv_pread(bs->file, offset, &ext, sizeof(ext)) != sizeof(ext)) { - fprintf(stderr, "qcow_handle_extension: ERROR: " + fprintf(stderr, "qcow_read_extension: ERROR: " "pread fail from offset %" PRIu64 "\n", offset); return 1; @@ -106,10 +106,10 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset, printf("ext.magic = 0x%x\n", ext.magic); #endif switch (ext.magic) { - case QCOW_EXT_MAGIC_END: + case QCOW2_EXT_MAGIC_END: return 0; - case QCOW_EXT_MAGIC_BACKING_FORMAT: + case QCOW2_EXT_MAGIC_BACKING_FORMAT: if (ext.len >= sizeof(bs->backing_format)) { fprintf(stderr, "ERROR: ext_backing_format: len=%u too large" " (>=%zu)\n", @@ -137,7 +137,7 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset, } -static int qcow_open(BlockDriverState *bs, int flags) +static int qcow2_open(BlockDriverState *bs, int flags) { BDRVQcowState *s = bs->opaque; int len, i; @@ -222,7 +222,7 @@ static int qcow_open(BlockDriverState *bs, int flags) ext_end = header.backing_file_offset; else ext_end = s->cluster_size; - if (qcow_read_extensions(bs, sizeof(header), ext_end)) + if (qcow2_read_extensions(bs, sizeof(header), ext_end)) goto fail; /* read the backing file name */ @@ -252,7 +252,7 @@ static int qcow_open(BlockDriverState *bs, int flags) return -1; } -static int qcow_set_key(BlockDriverState *bs, const char *key) +static int qcow2_set_key(BlockDriverState *bs, const char *key) { BDRVQcowState *s = bs->opaque; uint8_t keybuf[16]; @@ -294,8 +294,8 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) return 0; } -static int qcow_is_allocated(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, int *pnum) +static int qcow2_is_allocated(BlockDriverState *bs, int64_t sector_num, + int nb_sectors, int *pnum) { uint64_t cluster_offset; int ret; @@ -313,7 +313,7 @@ static int qcow_is_allocated(BlockDriverState *bs, int64_t sector_num, /* handle reading after the end of the backing file */ int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov, - int64_t sector_num, int nb_sectors) + int64_t sector_num, int nb_sectors) { int n1; if ((sector_num + nb_sectors) <= bs->total_sectors) @@ -344,7 +344,7 @@ typedef struct QCowAIOCB { QLIST_ENTRY(QCowAIOCB) next_depend; } QCowAIOCB; -static void qcow_aio_cancel(BlockDriverAIOCB *blockacb) +static void qcow2_aio_cancel(BlockDriverAIOCB *blockacb) { QCowAIOCB *acb = container_of(blockacb, QCowAIOCB, common); if (acb->hd_aiocb) @@ -352,21 +352,21 @@ static void qcow_aio_cancel(BlockDriverAIOCB *blockacb) qemu_aio_release(acb); } -static AIOPool qcow_aio_pool = { +static AIOPool qcow2_aio_pool = { .aiocb_size = sizeof(QCowAIOCB), - .cancel = qcow_aio_cancel, + .cancel = qcow2_aio_cancel, }; -static void qcow_aio_read_cb(void *opaque, int ret); -static void qcow_aio_read_bh(void *opaque) +static void qcow2_aio_read_cb(void *opaque, int ret); +static void qcow2_aio_read_bh(void *opaque) { QCowAIOCB *acb = opaque; qemu_bh_delete(acb->bh); acb->bh = NULL; - qcow_aio_read_cb(opaque, 0); + qcow2_aio_read_cb(opaque, 0); } -static int qcow_schedule_bh(QEMUBHFunc *cb, QCowAIOCB *acb) +static int qcow2_schedule_bh(QEMUBHFunc *cb, QCowAIOCB *acb) { if (acb->bh) return -EIO; @@ -380,7 +380,7 @@ static int qcow_schedule_bh(QEMUBHFunc *cb, QCowAIOCB *acb) return 0; } -static void qcow_aio_read_cb(void *opaque, int ret) +static void qcow2_aio_read_cb(void *opaque, int ret) { QCowAIOCB *acb = opaque; BlockDriverState *bs = acb->common.bs; @@ -399,10 +399,11 @@ static void qcow_aio_read_cb(void *opaque, int ret) } else { if (s->crypt_method) { qcow2_encrypt_sectors(s, acb->sector_num, acb->cluster_data, - acb->cluster_data, acb->cur_nr_sectors, 0, &s->aes_decrypt_key); + acb->cluster_data, acb->cur_nr_sectors, + 0, &s->aes_decrypt_key); qemu_iovec_reset(&acb->hd_qiov); qemu_iovec_copy(&acb->hd_qiov, acb->qiov, acb->bytes_done, - acb->cur_nr_sectors * 512); + acb->cur_nr_sectors * 512); qemu_iovec_from_buffer(&acb->hd_qiov, acb->cluster_data, 512 * acb->cur_nr_sectors); } @@ -426,7 +427,7 @@ static void qcow_aio_read_cb(void *opaque, int ret) } ret = qcow2_get_cluster_offset(bs, acb->sector_num << 9, - &acb->cur_nr_sectors, &acb->cluster_offset); + &acb->cur_nr_sectors, &acb->cluster_offset); if (ret < 0) { goto done; } @@ -442,23 +443,23 @@ static void qcow_aio_read_cb(void *opaque, int ret) if (bs->backing_hd) { /* read from the base image */ n1 = qcow2_backing_read1(bs->backing_hd, &acb->hd_qiov, - acb->sector_num, acb->cur_nr_sectors); + acb->sector_num, acb->cur_nr_sectors); if (n1 > 0) { BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO); acb->hd_aiocb = bdrv_aio_readv(bs->backing_hd, acb->sector_num, &acb->hd_qiov, acb->cur_nr_sectors, - qcow_aio_read_cb, acb); + qcow2_aio_read_cb, acb); if (acb->hd_aiocb == NULL) goto done; } else { - ret = qcow_schedule_bh(qcow_aio_read_bh, acb); + ret = qcow2_schedule_bh(qcow2_aio_read_bh, acb); if (ret < 0) goto done; } } else { /* Note: in this case, no need to wait */ qemu_iovec_memset(&acb->hd_qiov, 0, 512 * acb->cur_nr_sectors); - ret = qcow_schedule_bh(qcow_aio_read_bh, acb); + ret = qcow2_schedule_bh(qcow2_aio_read_bh, acb); if (ret < 0) goto done; } @@ -471,7 +472,7 @@ static void qcow_aio_read_cb(void *opaque, int ret) s->cluster_cache + index_in_cluster * 512, 512 * acb->cur_nr_sectors); - ret = qcow_schedule_bh(qcow_aio_read_bh, acb); + ret = qcow2_schedule_bh(qcow2_aio_read_bh, acb); if (ret < 0) goto done; } else { @@ -501,7 +502,7 @@ static void qcow_aio_read_cb(void *opaque, int ret) acb->hd_aiocb = bdrv_aio_readv(bs->file, (acb->cluster_offset >> 9) + index_in_cluster, &acb->hd_qiov, acb->cur_nr_sectors, - qcow_aio_read_cb, acb); + qcow2_aio_read_cb, acb); if (acb->hd_aiocb == NULL) { ret = -EIO; goto done; @@ -515,13 +516,14 @@ done: qemu_aio_release(acb); } -static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs, - int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, - BlockDriverCompletionFunc *cb, void *opaque, int is_write) +static QCowAIOCB *qcow2_aio_setup(BlockDriverState *bs, int64_t sector_num, + QEMUIOVector *qiov, int nb_sectors, + BlockDriverCompletionFunc *cb, + void *opaque, int is_write) { QCowAIOCB *acb; - acb = qemu_aio_get(&qcow_aio_pool, bs, cb, opaque); + acb = qemu_aio_get(&qcow2_aio_pool, bs, cb, opaque); if (!acb) return NULL; acb->hd_aiocb = NULL; @@ -539,21 +541,23 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs, return acb; } -static BlockDriverAIOCB *qcow_aio_readv(BlockDriverState *bs, - int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, - BlockDriverCompletionFunc *cb, void *opaque) +static BlockDriverAIOCB *qcow2_aio_readv(BlockDriverState *bs, + int64_t sector_num, + QEMUIOVector *qiov, int nb_sectors, + BlockDriverCompletionFunc *cb, + void *opaque) { QCowAIOCB *acb; - acb = qcow_aio_setup(bs, sector_num, qiov, nb_sectors, cb, opaque, 0); + acb = qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, cb, opaque, 0); if (!acb) return NULL; - qcow_aio_read_cb(acb, 0); + qcow2_aio_read_cb(acb, 0); return &acb->common; } -static void qcow_aio_write_cb(void *opaque, int ret); +static void qcow2_aio_write_cb(void *opaque, int ret); static void run_dependent_requests(QCowL2Meta *m) { @@ -567,14 +571,14 @@ static void run_dependent_requests(QCowL2Meta *m) /* Restart all dependent requests */ QLIST_FOREACH_SAFE(req, &m->dependent_requests, next_depend, next) { - qcow_aio_write_cb(req, 0); + qcow2_aio_write_cb(req, 0); } /* Empty the list for the next part of the request */ QLIST_INIT(&m->dependent_requests); } -static void qcow_aio_write_cb(void *opaque, int ret) +static void qcow2_aio_write_cb(void *opaque, int ret) { QCowAIOCB *acb = opaque; BlockDriverState *bs = acb->common.bs; @@ -640,7 +644,8 @@ static void qcow_aio_write_cb(void *opaque, int ret) qemu_iovec_to_buffer(&acb->hd_qiov, acb->cluster_data); qcow2_encrypt_sectors(s, acb->sector_num, acb->cluster_data, - acb->cluster_data, acb->cur_nr_sectors, 1, &s->aes_encrypt_key); + acb->cluster_data, acb->cur_nr_sectors, + 1, &s->aes_encrypt_key); qemu_iovec_reset(&acb->hd_qiov); qemu_iovec_add(&acb->hd_qiov, acb->cluster_data, @@ -651,7 +656,7 @@ static void qcow_aio_write_cb(void *opaque, int ret) acb->hd_aiocb = bdrv_aio_writev(bs->file, (acb->cluster_offset >> 9) + index_in_cluster, &acb->hd_qiov, acb->cur_nr_sectors, - qcow_aio_write_cb, acb); + qcow2_aio_write_cb, acb); if (acb->hd_aiocb == NULL) { ret = -EIO; goto fail; @@ -669,24 +674,26 @@ done: qemu_aio_release(acb); } -static BlockDriverAIOCB *qcow_aio_writev(BlockDriverState *bs, - int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, - BlockDriverCompletionFunc *cb, void *opaque) +static BlockDriverAIOCB *qcow2_aio_writev(BlockDriverState *bs, + int64_t sector_num, + QEMUIOVector *qiov, int nb_sectors, + BlockDriverCompletionFunc *cb, + void *opaque) { BDRVQcowState *s = bs->opaque; QCowAIOCB *acb; s->cluster_cache_offset = -1; /* disable compressed cache */ - acb = qcow_aio_setup(bs, sector_num, qiov, nb_sectors, cb, opaque, 1); + acb = qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, cb, opaque, 1); if (!acb) return NULL; - qcow_aio_write_cb(acb, 0); + qcow2_aio_write_cb(acb, 0); return &acb->common; } -static void qcow_close(BlockDriverState *bs) +static void qcow2_close(BlockDriverState *bs) { BDRVQcowState *s = bs->opaque; qemu_free(s->l1_table); @@ -705,7 +712,8 @@ static void qcow_close(BlockDriverState *bs) * Returns 0 on success, -errno in error cases. */ static int qcow2_update_ext_header(BlockDriverState *bs, - const char *backing_file, const char *backing_fmt) + const char *backing_file, + const char *backing_fmt) { size_t backing_file_len = 0; size_t backing_fmt_len = 0; @@ -721,7 +729,7 @@ static int qcow2_update_ext_header(BlockDriverState *bs, /* Prepare the backing file format extension if needed */ if (backing_fmt) { ext_backing_fmt.len = cpu_to_be32(strlen(backing_fmt)); - ext_backing_fmt.magic = cpu_to_be32(QCOW_EXT_MAGIC_BACKING_FORMAT); + ext_backing_fmt.magic = cpu_to_be32(QCOW2_EXT_MAGIC_BACKING_FORMAT); backing_fmt_len = ((sizeof(ext_backing_fmt) + strlen(backing_fmt) + 7) & ~7); } @@ -790,7 +798,8 @@ fail: } static int qcow2_change_backing_file(BlockDriverState *bs, - const char *backing_file, const char *backing_fmt) + const char *backing_file, + const char *backing_fmt) { return qcow2_update_ext_header(bs, backing_file, backing_fmt); } @@ -848,10 +857,10 @@ static int preallocate(BlockDriverState *bs) return 0; } -static int qcow_create2(const char *filename, int64_t total_size, - const char *backing_file, const char *backing_format, - int flags, size_t cluster_size, int prealloc, - QEMUOptionParameter *options) +static int qcow2_create2(const char *filename, int64_t total_size, + const char *backing_file, const char *backing_format, + int flags, size_t cluster_size, int prealloc, + QEMUOptionParameter *options) { /* Calulate cluster_bits */ int cluster_bits; @@ -974,7 +983,7 @@ out: return ret; } -static int qcow_create(const char *filename, QEMUOptionParameter *options) +static int qcow2_create(const char *filename, QEMUOptionParameter *options) { const char *backing_file = NULL; const char *backing_fmt = NULL; @@ -1017,11 +1026,11 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) return -EINVAL; } - return qcow_create2(filename, sectors, backing_file, backing_fmt, flags, - cluster_size, prealloc, options); + return qcow2_create2(filename, sectors, backing_file, backing_fmt, flags, + cluster_size, prealloc, options); } -static int qcow_make_empty(BlockDriverState *bs) +static int qcow2_make_empty(BlockDriverState *bs) { #if 0 /* XXX: not correct */ @@ -1080,8 +1089,8 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset) /* XXX: put compressed sectors first, then all the cluster aligned tables to avoid losing bytes in alignment */ -static int qcow_write_compressed(BlockDriverState *bs, int64_t sector_num, - const uint8_t *buf, int nb_sectors) +static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num, + const uint8_t *buf, int nb_sectors) { BDRVQcowState *s = bs->opaque; z_stream strm; @@ -1132,8 +1141,8 @@ static int qcow_write_compressed(BlockDriverState *bs, int64_t sector_num, /* could not compress: write normal cluster */ bdrv_write(bs, sector_num, buf, s->cluster_sectors); } else { - cluster_offset = qcow2_alloc_compressed_cluster_offset(bs, - sector_num << 9, out_len); + cluster_offset = + qcow2_alloc_compressed_cluster_offset(bs, sector_num << 9, out_len); if (!cluster_offset) return -1; cluster_offset &= s->cluster_offset_mask; @@ -1148,32 +1157,33 @@ static int qcow_write_compressed(BlockDriverState *bs, int64_t sector_num, return 0; } -static int qcow_flush(BlockDriverState *bs) +static int qcow2_flush(BlockDriverState *bs) { return bdrv_flush(bs->file); } -static BlockDriverAIOCB *qcow_aio_flush(BlockDriverState *bs, - BlockDriverCompletionFunc *cb, void *opaque) +static BlockDriverAIOCB *qcow2_aio_flush(BlockDriverState *bs, + BlockDriverCompletionFunc *cb, + void *opaque) { return bdrv_aio_flush(bs->file, cb, opaque); } -static int64_t qcow_vm_state_offset(BDRVQcowState *s) +static int64_t qcow2_vm_state_offset(BDRVQcowState *s) { return (int64_t)s->l1_vm_state_index << (s->cluster_bits + s->l2_bits); } -static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) +static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) { BDRVQcowState *s = bs->opaque; bdi->cluster_size = s->cluster_size; - bdi->vm_state_offset = qcow_vm_state_offset(s); + bdi->vm_state_offset = qcow2_vm_state_offset(s); return 0; } -static int qcow_check(BlockDriverState *bs, BdrvCheckResult *result) +static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result) { return qcow2_check_refcounts(bs, result); } @@ -1199,8 +1209,8 @@ static void dump_refcounts(BlockDriverState *bs) } #endif -static int qcow_save_vmstate(BlockDriverState *bs, const uint8_t *buf, - int64_t pos, int size) +static int qcow2_save_vmstate(BlockDriverState *bs, const uint8_t *buf, + int64_t pos, int size) { BDRVQcowState *s = bs->opaque; int growable = bs->growable; @@ -1208,14 +1218,14 @@ static int qcow_save_vmstate(BlockDriverState *bs, const uint8_t *buf, BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE); bs->growable = 1; - ret = bdrv_pwrite(bs, qcow_vm_state_offset(s) + pos, buf, size); + ret = bdrv_pwrite(bs, qcow2_vm_state_offset(s) + pos, buf, size); bs->growable = growable; return ret; } -static int qcow_load_vmstate(BlockDriverState *bs, uint8_t *buf, - int64_t pos, int size) +static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf, + int64_t pos, int size) { BDRVQcowState *s = bs->opaque; int growable = bs->growable; @@ -1223,13 +1233,13 @@ static int qcow_load_vmstate(BlockDriverState *bs, uint8_t *buf, BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD); bs->growable = 1; - ret = bdrv_pread(bs, qcow_vm_state_offset(s) + pos, buf, size); + ret = bdrv_pread(bs, qcow2_vm_state_offset(s) + pos, buf, size); bs->growable = growable; return ret; } -static QEMUOptionParameter qcow_create_options[] = { +static QEMUOptionParameter qcow2_create_options[] = { { .name = BLOCK_OPT_SIZE, .type = OPT_SIZE, @@ -1266,36 +1276,36 @@ static QEMUOptionParameter qcow_create_options[] = { static BlockDriver bdrv_qcow2 = { .format_name = "qcow2", .instance_size = sizeof(BDRVQcowState), - .bdrv_probe = qcow_probe, - .bdrv_open = qcow_open, - .bdrv_close = qcow_close, - .bdrv_create = qcow_create, - .bdrv_flush = qcow_flush, - .bdrv_is_allocated = qcow_is_allocated, - .bdrv_set_key = qcow_set_key, - .bdrv_make_empty = qcow_make_empty, - - .bdrv_aio_readv = qcow_aio_readv, - .bdrv_aio_writev = qcow_aio_writev, - .bdrv_aio_flush = qcow_aio_flush, + .bdrv_probe = qcow2_probe, + .bdrv_open = qcow2_open, + .bdrv_close = qcow2_close, + .bdrv_create = qcow2_create, + .bdrv_flush = qcow2_flush, + .bdrv_is_allocated = qcow2_is_allocated, + .bdrv_set_key = qcow2_set_key, + .bdrv_make_empty = qcow2_make_empty, + + .bdrv_aio_readv = qcow2_aio_readv, + .bdrv_aio_writev = qcow2_aio_writev, + .bdrv_aio_flush = qcow2_aio_flush, .bdrv_truncate = qcow2_truncate, - .bdrv_write_compressed = qcow_write_compressed, + .bdrv_write_compressed = qcow2_write_compressed, .bdrv_snapshot_create = qcow2_snapshot_create, .bdrv_snapshot_goto = qcow2_snapshot_goto, .bdrv_snapshot_delete = qcow2_snapshot_delete, .bdrv_snapshot_list = qcow2_snapshot_list, .bdrv_snapshot_load_tmp = qcow2_snapshot_load_tmp, - .bdrv_get_info = qcow_get_info, + .bdrv_get_info = qcow2_get_info, - .bdrv_save_vmstate = qcow_save_vmstate, - .bdrv_load_vmstate = qcow_load_vmstate, + .bdrv_save_vmstate = qcow2_save_vmstate, + .bdrv_load_vmstate = qcow2_load_vmstate, .bdrv_change_backing_file = qcow2_change_backing_file, - .create_options = qcow_create_options, - .bdrv_check = qcow_check, + .create_options = qcow2_create_options, + .bdrv_check = qcow2_check, }; static void bdrv_qcow2_init(void) -- 1.7.3.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH 1/2] block/qcow2.c: rename qcow_ functions to qcow2_ 2010-12-16 16:05 ` [Qemu-devel] [PATCH 1/2] block/qcow2.c: rename qcow_ functions to qcow2_ Jes.Sorensen @ 2010-12-17 14:20 ` Kevin Wolf 2010-12-17 14:37 ` Jes Sorensen 0 siblings, 1 reply; 8+ messages in thread From: Kevin Wolf @ 2010-12-17 14:20 UTC (permalink / raw) To: Jes.Sorensen; +Cc: qemu-devel Am 16.12.2010 17:05, schrieb Jes.Sorensen@redhat.com: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > It doesn't really make sense for functions in qcow2.c to be named > qcow_ so convert the names to match correctly. > > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> > --- > block/qcow2-cluster.c | 6 +- > block/qcow2-snapshot.c | 6 +- > block/qcow2.c | 210 +++++++++++++++++++++++++----------------------- > 3 files changed, 116 insertions(+), 106 deletions(-) > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > index b040208..6928c63 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -352,8 +352,8 @@ void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, > } > > > -static int qcow_read(BlockDriverState *bs, int64_t sector_num, > - uint8_t *buf, int nb_sectors) > +static int qcow2_read(BlockDriverState *bs, int64_t sector_num, > + uint8_t *buf, int nb_sectors) > { > BDRVQcowState *s = bs->opaque; > int ret, index_in_cluster, n, n1; > @@ -419,7 +419,7 @@ static int copy_sectors(BlockDriverState *bs, uint64_t start_sect, > if (n <= 0) > return 0; > BLKDBG_EVENT(bs->file, BLKDBG_COW_READ); > - ret = qcow_read(bs, start_sect + n_start, s->cluster_data, n); > + ret = qcow2_read(bs, start_sect + n_start, s->cluster_data, n); > if (ret < 0) > return ret; > if (s->crypt_method) { > diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c > index aacf357..74823a5 100644 > --- a/block/qcow2-snapshot.c > +++ b/block/qcow2-snapshot.c > @@ -116,7 +116,7 @@ int qcow2_read_snapshots(BlockDriverState *bs) > } > > /* add at the end of the file a new list of snapshots */ > -static int qcow_write_snapshots(BlockDriverState *bs) > +static int qcow2_write_snapshots(BlockDriverState *bs) > { > BDRVQcowState *s = bs->opaque; > QCowSnapshot *sn; > @@ -300,7 +300,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) > s->snapshots = snapshots1; > s->snapshots[s->nb_snapshots++] = *sn; > > - if (qcow_write_snapshots(bs) < 0) > + if (qcow2_write_snapshots(bs) < 0) > goto fail; > #ifdef DEBUG_ALLOC > qcow2_check_refcounts(bs); > @@ -378,7 +378,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id) > qemu_free(sn->name); > memmove(sn, sn + 1, (s->nb_snapshots - snapshot_index - 1) * sizeof(*sn)); > s->nb_snapshots--; > - ret = qcow_write_snapshots(bs); > + ret = qcow2_write_snapshots(bs); > if (ret < 0) { > /* XXX: restore snapshot if error ? */ > return ret; > diff --git a/block/qcow2.c b/block/qcow2.c > index 537c479..d7fd167 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -50,10 +50,10 @@ typedef struct { > uint32_t magic; > uint32_t len; > } QCowExtension; > -#define QCOW_EXT_MAGIC_END 0 > -#define QCOW_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA > +#define QCOW2_EXT_MAGIC_END 0 > +#define QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA > > -static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename) > +static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename) > { > const QCowHeader *cow_header = (const void *)buf; > > @@ -73,14 +73,14 @@ static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename) > * unknown magic is skipped (future extension this version knows nothing about) > * return 0 upon success, non-0 otherwise > */ > -static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset, > - uint64_t end_offset) > +static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, > + uint64_t end_offset) > { > QCowExtension ext; > uint64_t offset; > > #ifdef DEBUG_EXT > - printf("qcow_read_extensions: start=%ld end=%ld\n", start_offset, end_offset); > + printf("qcow2_read_extensions: start=%ld end=%ld\n", start_offset, end_offset); > #endif > offset = start_offset; > while (offset < end_offset) { > @@ -88,13 +88,13 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset, > #ifdef DEBUG_EXT > /* Sanity check */ > if (offset > s->cluster_size) > - printf("qcow_handle_extension: suspicious offset %lu\n", offset); > + printf("qcow_read_extension: suspicious offset %lu\n", offset); It's now qcow2_read_extensions > > printf("attemting to read extended header in offset %lu\n", offset); > #endif > > if (bdrv_pread(bs->file, offset, &ext, sizeof(ext)) != sizeof(ext)) { > - fprintf(stderr, "qcow_handle_extension: ERROR: " > + fprintf(stderr, "qcow_read_extension: ERROR: " Same here. > "pread fail from offset %" PRIu64 "\n", > offset); > return 1; > @@ -106,10 +106,10 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset, > printf("ext.magic = 0x%x\n", ext.magic); > #endif > switch (ext.magic) { > - case QCOW_EXT_MAGIC_END: > + case QCOW2_EXT_MAGIC_END: > return 0; > > - case QCOW_EXT_MAGIC_BACKING_FORMAT: > + case QCOW2_EXT_MAGIC_BACKING_FORMAT: > if (ext.len >= sizeof(bs->backing_format)) { > fprintf(stderr, "ERROR: ext_backing_format: len=%u too large" > " (>=%zu)\n", > @@ -137,7 +137,7 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset, > } > > > -static int qcow_open(BlockDriverState *bs, int flags) > +static int qcow2_open(BlockDriverState *bs, int flags) > { > BDRVQcowState *s = bs->opaque; > int len, i; > @@ -222,7 +222,7 @@ static int qcow_open(BlockDriverState *bs, int flags) > ext_end = header.backing_file_offset; > else > ext_end = s->cluster_size; > - if (qcow_read_extensions(bs, sizeof(header), ext_end)) > + if (qcow2_read_extensions(bs, sizeof(header), ext_end)) > goto fail; > > /* read the backing file name */ > @@ -252,7 +252,7 @@ static int qcow_open(BlockDriverState *bs, int flags) > return -1; > } > > -static int qcow_set_key(BlockDriverState *bs, const char *key) > +static int qcow2_set_key(BlockDriverState *bs, const char *key) > { > BDRVQcowState *s = bs->opaque; > uint8_t keybuf[16]; > @@ -294,8 +294,8 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) > return 0; > } > > -static int qcow_is_allocated(BlockDriverState *bs, int64_t sector_num, > - int nb_sectors, int *pnum) > +static int qcow2_is_allocated(BlockDriverState *bs, int64_t sector_num, > + int nb_sectors, int *pnum) > { > uint64_t cluster_offset; > int ret; > @@ -313,7 +313,7 @@ static int qcow_is_allocated(BlockDriverState *bs, int64_t sector_num, > > /* handle reading after the end of the backing file */ > int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov, > - int64_t sector_num, int nb_sectors) > + int64_t sector_num, int nb_sectors) This isn't related to renaming functions. Please don't include pure formatting changes, all they do is making git blame work worse. > { > int n1; > if ((sector_num + nb_sectors) <= bs->total_sectors) > @@ -344,7 +344,7 @@ typedef struct QCowAIOCB { > QLIST_ENTRY(QCowAIOCB) next_depend; > } QCowAIOCB; > > -static void qcow_aio_cancel(BlockDriverAIOCB *blockacb) > +static void qcow2_aio_cancel(BlockDriverAIOCB *blockacb) > { > QCowAIOCB *acb = container_of(blockacb, QCowAIOCB, common); > if (acb->hd_aiocb) > @@ -352,21 +352,21 @@ static void qcow_aio_cancel(BlockDriverAIOCB *blockacb) > qemu_aio_release(acb); > } > > -static AIOPool qcow_aio_pool = { > +static AIOPool qcow2_aio_pool = { > .aiocb_size = sizeof(QCowAIOCB), > - .cancel = qcow_aio_cancel, > + .cancel = qcow2_aio_cancel, > }; > > -static void qcow_aio_read_cb(void *opaque, int ret); > -static void qcow_aio_read_bh(void *opaque) > +static void qcow2_aio_read_cb(void *opaque, int ret); > +static void qcow2_aio_read_bh(void *opaque) > { > QCowAIOCB *acb = opaque; > qemu_bh_delete(acb->bh); > acb->bh = NULL; > - qcow_aio_read_cb(opaque, 0); > + qcow2_aio_read_cb(opaque, 0); > } > > -static int qcow_schedule_bh(QEMUBHFunc *cb, QCowAIOCB *acb) > +static int qcow2_schedule_bh(QEMUBHFunc *cb, QCowAIOCB *acb) > { > if (acb->bh) > return -EIO; > @@ -380,7 +380,7 @@ static int qcow_schedule_bh(QEMUBHFunc *cb, QCowAIOCB *acb) > return 0; > } > > -static void qcow_aio_read_cb(void *opaque, int ret) > +static void qcow2_aio_read_cb(void *opaque, int ret) > { > QCowAIOCB *acb = opaque; > BlockDriverState *bs = acb->common.bs; > @@ -399,10 +399,11 @@ static void qcow_aio_read_cb(void *opaque, int ret) > } else { > if (s->crypt_method) { > qcow2_encrypt_sectors(s, acb->sector_num, acb->cluster_data, > - acb->cluster_data, acb->cur_nr_sectors, 0, &s->aes_decrypt_key); > + acb->cluster_data, acb->cur_nr_sectors, > + 0, &s->aes_decrypt_key); Same here, plus the old version wasn't obviously indented wrong, but just not according to your personal style. The following changes include more lines that need not be changed for the rename and just change the coding style (even though CODING_STYLE doesn't make a statement on this, so the old version isn't wrong). Please leave them out. > @@ -1266,36 +1276,36 @@ static QEMUOptionParameter qcow_create_options[] = { > static BlockDriver bdrv_qcow2 = { > .format_name = "qcow2", > .instance_size = sizeof(BDRVQcowState), > - .bdrv_probe = qcow_probe, > - .bdrv_open = qcow_open, > - .bdrv_close = qcow_close, > - .bdrv_create = qcow_create, > - .bdrv_flush = qcow_flush, > - .bdrv_is_allocated = qcow_is_allocated, > - .bdrv_set_key = qcow_set_key, > - .bdrv_make_empty = qcow_make_empty, > - > - .bdrv_aio_readv = qcow_aio_readv, > - .bdrv_aio_writev = qcow_aio_writev, > - .bdrv_aio_flush = qcow_aio_flush, > + .bdrv_probe = qcow2_probe, > + .bdrv_open = qcow2_open, > + .bdrv_close = qcow2_close, > + .bdrv_create = qcow2_create, > + .bdrv_flush = qcow2_flush, > + .bdrv_is_allocated = qcow2_is_allocated, > + .bdrv_set_key = qcow2_set_key, > + .bdrv_make_empty = qcow2_make_empty, Can you take the chance and convert the tabs to spaces? Kevin ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH 1/2] block/qcow2.c: rename qcow_ functions to qcow2_ 2010-12-17 14:20 ` [Qemu-devel] " Kevin Wolf @ 2010-12-17 14:37 ` Jes Sorensen 0 siblings, 0 replies; 8+ messages in thread From: Jes Sorensen @ 2010-12-17 14:37 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel On 12/17/10 15:20, Kevin Wolf wrote: >> offset = start_offset; >> while (offset < end_offset) { >> @@ -88,13 +88,13 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset, >> #ifdef DEBUG_EXT >> /* Sanity check */ >> if (offset > s->cluster_size) >> - printf("qcow_handle_extension: suspicious offset %lu\n", offset); >> + printf("qcow_read_extension: suspicious offset %lu\n", offset); > > It's now qcow2_read_extensions Fixed >> @@ -313,7 +313,7 @@ static int qcow_is_allocated(BlockDriverState *bs, int64_t sector_num, >> >> /* handle reading after the end of the backing file */ >> int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov, >> - int64_t sector_num, int nb_sectors) >> + int64_t sector_num, int nb_sectors) > > This isn't related to renaming functions. Please don't include pure > formatting changes, all they do is making git blame work worse. No it makes the formatting consistent with the rest of the functions in the file. I can leave it out, but then we just have more ugliness in the file. >> @@ -399,10 +399,11 @@ static void qcow_aio_read_cb(void *opaque, int ret) >> } else { >> if (s->crypt_method) { >> qcow2_encrypt_sectors(s, acb->sector_num, acb->cluster_data, >> - acb->cluster_data, acb->cur_nr_sectors, 0, &s->aes_decrypt_key); >> + acb->cluster_data, acb->cur_nr_sectors, >> + 0, &s->aes_decrypt_key); > > Same here, plus the old version wasn't obviously indented wrong, but > just not according to your personal style. Sorry it's broken formatting. But sure, I'll put it back to being unreadable. > The following changes include more lines that need not be changed for > the rename and just change the coding style (even though CODING_STYLE > doesn't make a statement on this, so the old version isn't wrong). > Please leave them out. Actually that is in the patch, I did a pure search replace, no formatting. But I've fixed it. Jes ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2] Add proper -errno error return values to qcow2_open() 2010-12-16 16:05 [Qemu-devel] [PATCH 0/2] qcow2 cleanups Jes.Sorensen 2010-12-16 16:05 ` [Qemu-devel] [PATCH 1/2] block/qcow2.c: rename qcow_ functions to qcow2_ Jes.Sorensen @ 2010-12-16 16:05 ` Jes.Sorensen 2010-12-17 14:27 ` [Qemu-devel] " Kevin Wolf 1 sibling, 1 reply; 8+ messages in thread From: Jes.Sorensen @ 2010-12-16 16:05 UTC (permalink / raw) To: kwolf; +Cc: qemu-devel From: Jes Sorensen <Jes.Sorensen@redhat.com> In addition this adds missing braces to the function to be consistent with the coding style. Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> --- block/qcow2.c | 61 ++++++++++++++++++++++++++++++++++++++++---------------- 1 files changed, 43 insertions(+), 18 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index d7fd167..b4a9e5e 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -140,12 +140,14 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, static int qcow2_open(BlockDriverState *bs, int flags) { BDRVQcowState *s = bs->opaque; - int len, i; + int len, i, ret = 0; QCowHeader header; uint64_t ext_end; - if (bdrv_pread(bs->file, 0, &header, sizeof(header)) != sizeof(header)) + if (bdrv_pread(bs->file, 0, &header, sizeof(header)) != sizeof(header)) { + ret = -EIO; goto fail; + } be32_to_cpus(&header.magic); be32_to_cpus(&header.version); be64_to_cpus(&header.backing_file_offset); @@ -160,16 +162,23 @@ static int qcow2_open(BlockDriverState *bs, int flags) be64_to_cpus(&header.snapshots_offset); be32_to_cpus(&header.nb_snapshots); - if (header.magic != QCOW_MAGIC || header.version != QCOW_VERSION) + if (header.magic != QCOW_MAGIC || header.version != QCOW_VERSION) { + ret = -EINVAL; goto fail; + } if (header.cluster_bits < MIN_CLUSTER_BITS || - header.cluster_bits > MAX_CLUSTER_BITS) + header.cluster_bits > MAX_CLUSTER_BITS) { + ret = -EINVAL; goto fail; - if (header.crypt_method > QCOW_CRYPT_AES) + } + if (header.crypt_method > QCOW_CRYPT_AES) { + ret = -EINVAL; goto fail; + } s->crypt_method_header = header.crypt_method; - if (s->crypt_method_header) + if (s->crypt_method_header) { bs->encrypted = 1; + } s->cluster_bits = header.cluster_bits; s->cluster_size = 1 << s->cluster_bits; s->cluster_sectors = 1 << (s->cluster_bits - 9); @@ -191,15 +200,20 @@ static int qcow2_open(BlockDriverState *bs, int flags) s->l1_vm_state_index = size_to_l1(s, header.size); /* the L1 table must contain at least enough entries to put header.size bytes */ - if (s->l1_size < s->l1_vm_state_index) + if (s->l1_size < s->l1_vm_state_index) { + ret = -EINVAL; goto fail; + } s->l1_table_offset = header.l1_table_offset; if (s->l1_size > 0) { s->l1_table = qemu_mallocz( align_offset(s->l1_size * sizeof(uint64_t), 512)); - if (bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, s->l1_size * sizeof(uint64_t)) != - s->l1_size * sizeof(uint64_t)) + if (bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, + s->l1_size * sizeof(uint64_t)) != + s->l1_size * sizeof(uint64_t)) { + ret = -EIO; goto fail; + } for(i = 0;i < s->l1_size; i++) { be64_to_cpus(&s->l1_table[i]); } @@ -212,35 +226,46 @@ static int qcow2_open(BlockDriverState *bs, int flags) + 512); s->cluster_cache_offset = -1; - if (qcow2_refcount_init(bs) < 0) + ret = qcow2_refcount_init(bs); + if (ret != 0) { goto fail; + } QLIST_INIT(&s->cluster_allocs); /* read qcow2 extensions */ - if (header.backing_file_offset) + if (header.backing_file_offset) { ext_end = header.backing_file_offset; - else + } else { ext_end = s->cluster_size; - if (qcow2_read_extensions(bs, sizeof(header), ext_end)) + } + if (qcow2_read_extensions(bs, sizeof(header), ext_end)) { + ret = -EINVAL; goto fail; + } /* read the backing file name */ if (header.backing_file_offset != 0) { len = header.backing_file_size; - if (len > 1023) + if (len > 1023) { len = 1023; - if (bdrv_pread(bs->file, header.backing_file_offset, bs->backing_file, len) != len) + } + if (bdrv_pread(bs->file, header.backing_file_offset, + bs->backing_file, len) != len) { + ret = -EIO; goto fail; + } bs->backing_file[len] = '\0'; } - if (qcow2_read_snapshots(bs) < 0) + if (qcow2_read_snapshots(bs) < 0) { + ret = -EINVAL; goto fail; + } #ifdef DEBUG_ALLOC qcow2_check_refcounts(bs); #endif - return 0; + return ret; fail: qcow2_free_snapshots(bs); @@ -249,7 +274,7 @@ static int qcow2_open(BlockDriverState *bs, int flags) qemu_free(s->l2_cache); qemu_free(s->cluster_cache); qemu_free(s->cluster_data); - return -1; + return ret; } static int qcow2_set_key(BlockDriverState *bs, const char *key) -- 1.7.3.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH 2/2] Add proper -errno error return values to qcow2_open() 2010-12-16 16:05 ` [Qemu-devel] [PATCH 2/2] Add proper -errno error return values to qcow2_open() Jes.Sorensen @ 2010-12-17 14:27 ` Kevin Wolf 2010-12-17 14:47 ` Jes Sorensen 0 siblings, 1 reply; 8+ messages in thread From: Kevin Wolf @ 2010-12-17 14:27 UTC (permalink / raw) To: Jes.Sorensen; +Cc: qemu-devel Am 16.12.2010 17:05, schrieb Jes.Sorensen@redhat.com: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > In addition this adds missing braces to the function to be consistent > with the coding style. > > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> > --- > block/qcow2.c | 61 ++++++++++++++++++++++++++++++++++++++++---------------- > 1 files changed, 43 insertions(+), 18 deletions(-) > > diff --git a/block/qcow2.c b/block/qcow2.c > index d7fd167..b4a9e5e 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -140,12 +140,14 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, > static int qcow2_open(BlockDriverState *bs, int flags) > { > BDRVQcowState *s = bs->opaque; > - int len, i; > + int len, i, ret = 0; > QCowHeader header; > uint64_t ext_end; > > - if (bdrv_pread(bs->file, 0, &header, sizeof(header)) != sizeof(header)) > + if (bdrv_pread(bs->file, 0, &header, sizeof(header)) != sizeof(header)) { > + ret = -EIO; > goto fail; > + } ret = bdrv_pread(...); if (ret < 0) { goto fail; } We have a specific error code, so why throw it away? > be32_to_cpus(&header.magic); > be32_to_cpus(&header.version); > be64_to_cpus(&header.backing_file_offset); > @@ -160,16 +162,23 @@ static int qcow2_open(BlockDriverState *bs, int flags) > be64_to_cpus(&header.snapshots_offset); > be32_to_cpus(&header.nb_snapshots); > > - if (header.magic != QCOW_MAGIC || header.version != QCOW_VERSION) > + if (header.magic != QCOW_MAGIC || header.version != QCOW_VERSION) { > + ret = -EINVAL; > goto fail; > + } > if (header.cluster_bits < MIN_CLUSTER_BITS || > - header.cluster_bits > MAX_CLUSTER_BITS) > + header.cluster_bits > MAX_CLUSTER_BITS) { > + ret = -EINVAL; > goto fail; > - if (header.crypt_method > QCOW_CRYPT_AES) > + } > + if (header.crypt_method > QCOW_CRYPT_AES) { > + ret = -EINVAL; > goto fail; > + } > s->crypt_method_header = header.crypt_method; > - if (s->crypt_method_header) > + if (s->crypt_method_header) { > bs->encrypted = 1; > + } > s->cluster_bits = header.cluster_bits; > s->cluster_size = 1 << s->cluster_bits; > s->cluster_sectors = 1 << (s->cluster_bits - 9); > @@ -191,15 +200,20 @@ static int qcow2_open(BlockDriverState *bs, int flags) > s->l1_vm_state_index = size_to_l1(s, header.size); > /* the L1 table must contain at least enough entries to put > header.size bytes */ > - if (s->l1_size < s->l1_vm_state_index) > + if (s->l1_size < s->l1_vm_state_index) { > + ret = -EINVAL; > goto fail; > + } > s->l1_table_offset = header.l1_table_offset; > if (s->l1_size > 0) { > s->l1_table = qemu_mallocz( > align_offset(s->l1_size * sizeof(uint64_t), 512)); > - if (bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, s->l1_size * sizeof(uint64_t)) != > - s->l1_size * sizeof(uint64_t)) > + if (bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, > + s->l1_size * sizeof(uint64_t)) != > + s->l1_size * sizeof(uint64_t)) { > + ret = -EIO; > goto fail; > + } Same here. > for(i = 0;i < s->l1_size; i++) { > be64_to_cpus(&s->l1_table[i]); > } > @@ -212,35 +226,46 @@ static int qcow2_open(BlockDriverState *bs, int flags) > + 512); > s->cluster_cache_offset = -1; > > - if (qcow2_refcount_init(bs) < 0) > + ret = qcow2_refcount_init(bs); > + if (ret != 0) { > goto fail; > + } > > QLIST_INIT(&s->cluster_allocs); > > /* read qcow2 extensions */ > - if (header.backing_file_offset) > + if (header.backing_file_offset) { > ext_end = header.backing_file_offset; > - else > + } else { > ext_end = s->cluster_size; > - if (qcow2_read_extensions(bs, sizeof(header), ext_end)) > + } > + if (qcow2_read_extensions(bs, sizeof(header), ext_end)) { > + ret = -EINVAL; > goto fail; > + } > > /* read the backing file name */ > if (header.backing_file_offset != 0) { > len = header.backing_file_size; > - if (len > 1023) > + if (len > 1023) { > len = 1023; > - if (bdrv_pread(bs->file, header.backing_file_offset, bs->backing_file, len) != len) > + } > + if (bdrv_pread(bs->file, header.backing_file_offset, > + bs->backing_file, len) != len) { > + ret = -EIO; > goto fail; > + } And here. Otherwise the patch looks good to me. Kevin ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH 2/2] Add proper -errno error return values to qcow2_open() 2010-12-17 14:27 ` [Qemu-devel] " Kevin Wolf @ 2010-12-17 14:47 ` Jes Sorensen 0 siblings, 0 replies; 8+ messages in thread From: Jes Sorensen @ 2010-12-17 14:47 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel On 12/17/10 15:27, Kevin Wolf wrote: > Am 16.12.2010 17:05, schrieb Jes.Sorensen@redhat.com: >> From: Jes Sorensen <Jes.Sorensen@redhat.com> >> >> In addition this adds missing braces to the function to be consistent >> with the coding style. >> >> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> >> --- >> block/qcow2.c | 61 ++++++++++++++++++++++++++++++++++++++++---------------- >> 1 files changed, 43 insertions(+), 18 deletions(-) >> >> diff --git a/block/qcow2.c b/block/qcow2.c >> index d7fd167..b4a9e5e 100644 >> --- a/block/qcow2.c >> +++ b/block/qcow2.c >> @@ -140,12 +140,14 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, >> static int qcow2_open(BlockDriverState *bs, int flags) >> { >> BDRVQcowState *s = bs->opaque; >> - int len, i; >> + int len, i, ret = 0; >> QCowHeader header; >> uint64_t ext_end; >> >> - if (bdrv_pread(bs->file, 0, &header, sizeof(header)) != sizeof(header)) >> + if (bdrv_pread(bs->file, 0, &header, sizeof(header)) != sizeof(header)) { >> + ret = -EIO; >> goto fail; >> + } > > ret = bdrv_pread(...); > if (ret < 0) { > goto fail; > } Hmmm I must have confused something and looked at a wrong pread function where it just returned -1 on error. I'll fix it. Thanks, Jes ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 0/2] qcow2 cleanups @ 2010-12-17 15:02 Jes.Sorensen 2010-12-17 15:02 ` [Qemu-devel] [PATCH 2/2] Add proper -errno error return values to qcow2_open() Jes.Sorensen 0 siblings, 1 reply; 8+ messages in thread From: Jes.Sorensen @ 2010-12-17 15:02 UTC (permalink / raw) To: kwolf; +Cc: qemu-devel From: Jes Sorensen <Jes.Sorensen@redhat.com> Hi, These two patches tries to clean up the qcow2 code a little. First makes the function names consistent, ie. we shouldn't have qcow_ functions in the qcow2 code. Second tries to add proper errno return values to qcow2_open(). New in v2: Fix the bdrv_pread() handling as pointed out by Kevin. Fix error messages, and restore a couple of cases to their unreadable formatting to avoid formatting changes not directly related to the qcow_->qcow2_ rename. Jes Jes Sorensen (2): block/qcow2.c: rename qcow_ functions to qcow2_ Add proper -errno error return values to qcow2_open() block/qcow2-cluster.c | 6 +- block/qcow2-snapshot.c | 6 +- block/qcow2.c | 248 +++++++++++++++++++++++++++--------------------- 3 files changed, 145 insertions(+), 115 deletions(-) -- 1.7.3.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2] Add proper -errno error return values to qcow2_open() 2010-12-17 15:02 [Qemu-devel] [PATCH v2 0/2] qcow2 cleanups Jes.Sorensen @ 2010-12-17 15:02 ` Jes.Sorensen 0 siblings, 0 replies; 8+ messages in thread From: Jes.Sorensen @ 2010-12-17 15:02 UTC (permalink / raw) To: kwolf; +Cc: qemu-devel From: Jes Sorensen <Jes.Sorensen@redhat.com> In addition this adds missing braces to the function to be consistent with the coding style. Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> --- block/qcow2.c | 60 +++++++++++++++++++++++++++++++++++++++----------------- 1 files changed, 42 insertions(+), 18 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 4b41190..b6b094c 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -140,12 +140,14 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, static int qcow2_open(BlockDriverState *bs, int flags) { BDRVQcowState *s = bs->opaque; - int len, i; + int len, i, ret = 0; QCowHeader header; uint64_t ext_end; - if (bdrv_pread(bs->file, 0, &header, sizeof(header)) != sizeof(header)) + ret = bdrv_pread(bs->file, 0, &header, sizeof(header)); + if (ret < 0) { goto fail; + } be32_to_cpus(&header.magic); be32_to_cpus(&header.version); be64_to_cpus(&header.backing_file_offset); @@ -160,16 +162,23 @@ static int qcow2_open(BlockDriverState *bs, int flags) be64_to_cpus(&header.snapshots_offset); be32_to_cpus(&header.nb_snapshots); - if (header.magic != QCOW_MAGIC || header.version != QCOW_VERSION) + if (header.magic != QCOW_MAGIC || header.version != QCOW_VERSION) { + ret = -EINVAL; goto fail; + } if (header.cluster_bits < MIN_CLUSTER_BITS || - header.cluster_bits > MAX_CLUSTER_BITS) + header.cluster_bits > MAX_CLUSTER_BITS) { + ret = -EINVAL; goto fail; - if (header.crypt_method > QCOW_CRYPT_AES) + } + if (header.crypt_method > QCOW_CRYPT_AES) { + ret = -EINVAL; goto fail; + } s->crypt_method_header = header.crypt_method; - if (s->crypt_method_header) + if (s->crypt_method_header) { bs->encrypted = 1; + } s->cluster_bits = header.cluster_bits; s->cluster_size = 1 << s->cluster_bits; s->cluster_sectors = 1 << (s->cluster_bits - 9); @@ -191,15 +200,19 @@ static int qcow2_open(BlockDriverState *bs, int flags) s->l1_vm_state_index = size_to_l1(s, header.size); /* the L1 table must contain at least enough entries to put header.size bytes */ - if (s->l1_size < s->l1_vm_state_index) + if (s->l1_size < s->l1_vm_state_index) { + ret = -EINVAL; goto fail; + } s->l1_table_offset = header.l1_table_offset; if (s->l1_size > 0) { s->l1_table = qemu_mallocz( align_offset(s->l1_size * sizeof(uint64_t), 512)); - if (bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, s->l1_size * sizeof(uint64_t)) != - s->l1_size * sizeof(uint64_t)) + ret = bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, + s->l1_size * sizeof(uint64_t)); + if (ret < 0) { goto fail; + } for(i = 0;i < s->l1_size; i++) { be64_to_cpus(&s->l1_table[i]); } @@ -212,35 +225,46 @@ static int qcow2_open(BlockDriverState *bs, int flags) + 512); s->cluster_cache_offset = -1; - if (qcow2_refcount_init(bs) < 0) + ret = qcow2_refcount_init(bs); + if (ret != 0) { goto fail; + } QLIST_INIT(&s->cluster_allocs); /* read qcow2 extensions */ - if (header.backing_file_offset) + if (header.backing_file_offset) { ext_end = header.backing_file_offset; - else + } else { ext_end = s->cluster_size; - if (qcow2_read_extensions(bs, sizeof(header), ext_end)) + } + if (qcow2_read_extensions(bs, sizeof(header), ext_end)) { + ret = -EINVAL; goto fail; + } /* read the backing file name */ if (header.backing_file_offset != 0) { len = header.backing_file_size; - if (len > 1023) + if (len > 1023) { len = 1023; - if (bdrv_pread(bs->file, header.backing_file_offset, bs->backing_file, len) != len) + } + ret = bdrv_pread(bs->file, header.backing_file_offset, + bs->backing_file, len); + if (ret < 0) { goto fail; + } bs->backing_file[len] = '\0'; } - if (qcow2_read_snapshots(bs) < 0) + if (qcow2_read_snapshots(bs) < 0) { + ret = -EINVAL; goto fail; + } #ifdef DEBUG_ALLOC qcow2_check_refcounts(bs); #endif - return 0; + return ret; fail: qcow2_free_snapshots(bs); @@ -249,7 +273,7 @@ static int qcow2_open(BlockDriverState *bs, int flags) qemu_free(s->l2_cache); qemu_free(s->cluster_cache); qemu_free(s->cluster_data); - return -1; + return ret; } static int qcow2_set_key(BlockDriverState *bs, const char *key) -- 1.7.3.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-12-17 15:02 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-12-16 16:05 [Qemu-devel] [PATCH 0/2] qcow2 cleanups Jes.Sorensen 2010-12-16 16:05 ` [Qemu-devel] [PATCH 1/2] block/qcow2.c: rename qcow_ functions to qcow2_ Jes.Sorensen 2010-12-17 14:20 ` [Qemu-devel] " Kevin Wolf 2010-12-17 14:37 ` Jes Sorensen 2010-12-16 16:05 ` [Qemu-devel] [PATCH 2/2] Add proper -errno error return values to qcow2_open() Jes.Sorensen 2010-12-17 14:27 ` [Qemu-devel] " Kevin Wolf 2010-12-17 14:47 ` Jes Sorensen -- strict thread matches above, loose matches on Subject: below -- 2010-12-17 15:02 [Qemu-devel] [PATCH v2 0/2] qcow2 cleanups Jes.Sorensen 2010-12-17 15:02 ` [Qemu-devel] [PATCH 2/2] Add proper -errno error return values to qcow2_open() Jes.Sorensen
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).