Message ID | 1401882711-30508-11-git-send-email-armbru@redhat.com |
---|---|
State | New |
Headers | show |
The Wednesday 04 Jun 2014 à 13:51:51 (+0200), Markus Armbruster wrote : > bdrv_get_geometry() hides errors. Use bdrv_nb_sectors() or > bdrv_getlength() instead where that's obviously inappropriate. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Max Reitz <mreitz@redhat.com> > --- > block.c | 11 ++++++++--- > block/qapi.c | 14 +++++++++---- > qemu-img.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++------------- > 3 files changed, 68 insertions(+), 21 deletions(-) > > diff --git a/block.c b/block.c > index cae9085..b92f05f 100644 > --- a/block.c > +++ b/block.c > @@ -5574,7 +5574,7 @@ void bdrv_img_create(const char *filename, const char *fmt, > if (size && size->value.n == -1) { > if (backing_file && backing_file->value.s) { > BlockDriverState *bs; > - uint64_t size; > + int64_t size; > char buf[32]; > int back_flags; > > @@ -5593,8 +5593,13 @@ void bdrv_img_create(const char *filename, const char *fmt, > local_err = NULL; > goto out; > } > - bdrv_get_geometry(bs, &size); > - size *= 512; > + size = bdrv_getlength(bs); > + if (size < 0) { > + error_setg_errno(errp, -size, "Could not get size of '%s'", > + backing_file->value.s); > + bdrv_unref(bs); > + goto out; > + } > > snprintf(buf, sizeof(buf), "%" PRId64, size); > set_option_parameter(param, BLOCK_OPT_SIZE, buf); > diff --git a/block/qapi.c b/block/qapi.c > index 97e1641..90f406d 100644 > --- a/block/qapi.c > +++ b/block/qapi.c > @@ -165,19 +165,25 @@ void bdrv_query_image_info(BlockDriverState *bs, > ImageInfo **p_info, > Error **errp) > { > - uint64_t total_sectors; > + int64_t size; > const char *backing_filename; > char backing_filename2[1024]; > BlockDriverInfo bdi; > int ret; > Error *err = NULL; > - ImageInfo *info = g_new0(ImageInfo, 1); > + ImageInfo *info; > > - bdrv_get_geometry(bs, &total_sectors); > + size = bdrv_getlength(bs); > + if (size < 0) { > + error_setg_errno(errp, -size, "Can't get size of device '%s'", > + bdrv_get_device_name(bs)); > + return; > + } > > + info = g_new0(ImageInfo, 1); > info->filename = g_strdup(bs->filename); > info->format = g_strdup(bdrv_get_format_name(bs)); > - info->virtual_size = total_sectors * 512; > + info->virtual_size = size; > info->actual_size = bdrv_get_allocated_file_size(bs); > info->has_actual_size = info->actual_size >= 0; > if (bdrv_is_encrypted(bs)) { > diff --git a/qemu-img.c b/qemu-img.c > index e6d0edf..7e6dde0 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -958,7 +958,6 @@ static int img_compare(int argc, char **argv) > int64_t sector_num = 0; > int64_t nb_sectors; > int c, pnum; > - uint64_t bs_sectors; > uint64_t progress_base; > > for (;;) { > @@ -1020,10 +1019,20 @@ static int img_compare(int argc, char **argv) > > buf1 = qemu_blockalign(bs1, IO_BUF_SIZE); > buf2 = qemu_blockalign(bs2, IO_BUF_SIZE); > - bdrv_get_geometry(bs1, &bs_sectors); > - total_sectors1 = bs_sectors; > - bdrv_get_geometry(bs2, &bs_sectors); > - total_sectors2 = bs_sectors; > + total_sectors1 = bdrv_nb_sectors(bs1); > + if (total_sectors1 < 0) { > + error_report("Can't get size of %s: %s", > + filename1, strerror(-total_sectors1)); > + ret = 4; > + goto out; > + } > + total_sectors2 = bdrv_nb_sectors(bs2); > + if (total_sectors2 < 0) { > + error_report("Can't get size of %s: %s", > + filename2, strerror(-total_sectors2)); > + ret = 4; > + goto out; > + } > total_sectors = MIN(total_sectors1, total_sectors2); > progress_base = MAX(total_sectors1, total_sectors2); > > @@ -1185,7 +1194,7 @@ static int img_convert(int argc, char **argv) > BlockDriver *drv, *proto_drv; > BlockDriverState **bs = NULL, *out_bs = NULL; > int64_t total_sectors, nb_sectors, sector_num, bs_offset; > - uint64_t *bs_sectors = NULL; > + int64_t *bs_sectors = NULL; > uint8_t * buf = NULL; > size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE; > const uint8_t *buf1; > @@ -1326,7 +1335,7 @@ static int img_convert(int argc, char **argv) > qemu_progress_print(0, 100); > > bs = g_new0(BlockDriverState *, bs_n); > - bs_sectors = g_new(uint64_t, bs_n); > + bs_sectors = g_new(int64_t, bs_n); > > total_sectors = 0; > for (bs_i = 0; bs_i < bs_n; bs_i++) { > @@ -1340,7 +1349,13 @@ static int img_convert(int argc, char **argv) > ret = -1; > goto out; > } > - bdrv_get_geometry(bs[bs_i], &bs_sectors[bs_i]); > + bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]); > + if (bs_sectors[bs_i] < 0) { > + error_report("Could not get size of %s: %s", > + argv[optind + bs_i], strerror(-bs_sectors[bs_i])); > + ret = -1; > + goto out; > + } > total_sectors += bs_sectors[bs_i]; > } > > @@ -2421,9 +2436,9 @@ static int img_rebase(int argc, char **argv) > * the image is the same as the original one at any time. > */ > if (!unsafe) { > - uint64_t num_sectors; > - uint64_t old_backing_num_sectors; > - uint64_t new_backing_num_sectors = 0; > + int64_t num_sectors; > + int64_t old_backing_num_sectors; > + int64_t new_backing_num_sectors = 0; > uint64_t sector; > int n; > uint8_t * buf_old; > @@ -2433,10 +2448,31 @@ static int img_rebase(int argc, char **argv) > buf_old = qemu_blockalign(bs, IO_BUF_SIZE); > buf_new = qemu_blockalign(bs, IO_BUF_SIZE); > > - bdrv_get_geometry(bs, &num_sectors); > - bdrv_get_geometry(bs_old_backing, &old_backing_num_sectors); > + num_sectors = bdrv_nb_sectors(bs); > + if (num_sectors < 0) { > + error_report("Could not get size of '%s': %s", > + filename, strerror(-num_sectors)); > + ret = -1; > + goto out; > + } > + old_backing_num_sectors = bdrv_nb_sectors(bs_old_backing); > + if (old_backing_num_sectors < 0) { > + char backing_name[1024]; Could you put this on the heap ? I recently fixed a stack overflow when taking snapshots due to multiple PATH_MAX char array in a recursive function. We don't know how this function will be used later. Best regards Benoît > + > + bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name)); > + error_report("Could not get size of '%s': %s", > + backing_name, strerror(-old_backing_num_sectors)); > + ret = -1; > + goto out; > + } > if (bs_new_backing) { > - bdrv_get_geometry(bs_new_backing, &new_backing_num_sectors); > + new_backing_num_sectors = bdrv_nb_sectors(bs_new_backing); > + if (new_backing_num_sectors < 0) { > + error_report("Could not get size of '%s': %s", > + out_baseimg, strerror(-new_backing_num_sectors)); > + ret = -1; > + goto out; > + } > } > > if (num_sectors != 0) { > -- > 1.9.3 >
Benoît Canet <benoit.canet@irqsave.net> writes: > The Wednesday 04 Jun 2014 à 13:51:51 (+0200), Markus Armbruster wrote : >> bdrv_get_geometry() hides errors. Use bdrv_nb_sectors() or >> bdrv_getlength() instead where that's obviously inappropriate. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> Reviewed-by: Max Reitz <mreitz@redhat.com> [...] >> diff --git a/qemu-img.c b/qemu-img.c >> index e6d0edf..7e6dde0 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -958,7 +958,6 @@ static int img_compare(int argc, char **argv) >> int64_t sector_num = 0; >> int64_t nb_sectors; >> int c, pnum; >> - uint64_t bs_sectors; >> uint64_t progress_base; >> >> for (;;) { >> @@ -1020,10 +1019,20 @@ static int img_compare(int argc, char **argv) >> >> buf1 = qemu_blockalign(bs1, IO_BUF_SIZE); >> buf2 = qemu_blockalign(bs2, IO_BUF_SIZE); >> - bdrv_get_geometry(bs1, &bs_sectors); >> - total_sectors1 = bs_sectors; >> - bdrv_get_geometry(bs2, &bs_sectors); >> - total_sectors2 = bs_sectors; >> + total_sectors1 = bdrv_nb_sectors(bs1); >> + if (total_sectors1 < 0) { >> + error_report("Can't get size of %s: %s", >> + filename1, strerror(-total_sectors1)); >> + ret = 4; >> + goto out; >> + } >> + total_sectors2 = bdrv_nb_sectors(bs2); >> + if (total_sectors2 < 0) { >> + error_report("Can't get size of %s: %s", >> + filename2, strerror(-total_sectors2)); >> + ret = 4; >> + goto out; >> + } >> total_sectors = MIN(total_sectors1, total_sectors2); >> progress_base = MAX(total_sectors1, total_sectors2); >> >> @@ -1185,7 +1194,7 @@ static int img_convert(int argc, char **argv) >> BlockDriver *drv, *proto_drv; >> BlockDriverState **bs = NULL, *out_bs = NULL; >> int64_t total_sectors, nb_sectors, sector_num, bs_offset; >> - uint64_t *bs_sectors = NULL; >> + int64_t *bs_sectors = NULL; >> uint8_t * buf = NULL; >> size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE; >> const uint8_t *buf1; >> @@ -1326,7 +1335,7 @@ static int img_convert(int argc, char **argv) >> qemu_progress_print(0, 100); >> >> bs = g_new0(BlockDriverState *, bs_n); >> - bs_sectors = g_new(uint64_t, bs_n); >> + bs_sectors = g_new(int64_t, bs_n); >> >> total_sectors = 0; >> for (bs_i = 0; bs_i < bs_n; bs_i++) { >> @@ -1340,7 +1349,13 @@ static int img_convert(int argc, char **argv) >> ret = -1; >> goto out; >> } >> - bdrv_get_geometry(bs[bs_i], &bs_sectors[bs_i]); >> + bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]); >> + if (bs_sectors[bs_i] < 0) { >> + error_report("Could not get size of %s: %s", >> + argv[optind + bs_i], strerror(-bs_sectors[bs_i])); >> + ret = -1; >> + goto out; >> + } >> total_sectors += bs_sectors[bs_i]; >> } >> >> @@ -2421,9 +2436,9 @@ static int img_rebase(int argc, char **argv) >> * the image is the same as the original one at any time. >> */ >> if (!unsafe) { >> - uint64_t num_sectors; >> - uint64_t old_backing_num_sectors; >> - uint64_t new_backing_num_sectors = 0; >> + int64_t num_sectors; >> + int64_t old_backing_num_sectors; >> + int64_t new_backing_num_sectors = 0; >> uint64_t sector; >> int n; >> uint8_t * buf_old; >> @@ -2433,10 +2448,31 @@ static int img_rebase(int argc, char **argv) >> buf_old = qemu_blockalign(bs, IO_BUF_SIZE); >> buf_new = qemu_blockalign(bs, IO_BUF_SIZE); >> >> - bdrv_get_geometry(bs, &num_sectors); >> - bdrv_get_geometry(bs_old_backing, &old_backing_num_sectors); >> + num_sectors = bdrv_nb_sectors(bs); >> + if (num_sectors < 0) { >> + error_report("Could not get size of '%s': %s", >> + filename, strerror(-num_sectors)); >> + ret = -1; >> + goto out; >> + } >> + old_backing_num_sectors = bdrv_nb_sectors(bs_old_backing); >> + if (old_backing_num_sectors < 0) { >> + char backing_name[1024]; > > Could you put this on the heap ? > > I recently fixed a stack overflow when taking snapshots due to multiple PATH_MAX > char array in a recursive function. > > We don't know how this function will be used later. img_rebase() is not a general purpose function, it's a qemu-img command. Stack use is well below a single page even with my patch. I can't see how it could possibly become recursive.
The Wednesday 04 Jun 2014 à 15:20:18 (+0200), Markus Armbruster wrote : > Benoît Canet <benoit.canet@irqsave.net> writes: > > > The Wednesday 04 Jun 2014 à 13:51:51 (+0200), Markus Armbruster wrote : > >> bdrv_get_geometry() hides errors. Use bdrv_nb_sectors() or > >> bdrv_getlength() instead where that's obviously inappropriate. > >> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> Reviewed-by: Eric Blake <eblake@redhat.com> > >> Reviewed-by: Max Reitz <mreitz@redhat.com> > [...] > >> diff --git a/qemu-img.c b/qemu-img.c > >> index e6d0edf..7e6dde0 100644 > >> --- a/qemu-img.c > >> +++ b/qemu-img.c > >> @@ -958,7 +958,6 @@ static int img_compare(int argc, char **argv) > >> int64_t sector_num = 0; > >> int64_t nb_sectors; > >> int c, pnum; > >> - uint64_t bs_sectors; > >> uint64_t progress_base; > >> > >> for (;;) { > >> @@ -1020,10 +1019,20 @@ static int img_compare(int argc, char **argv) > >> > >> buf1 = qemu_blockalign(bs1, IO_BUF_SIZE); > >> buf2 = qemu_blockalign(bs2, IO_BUF_SIZE); > >> - bdrv_get_geometry(bs1, &bs_sectors); > >> - total_sectors1 = bs_sectors; > >> - bdrv_get_geometry(bs2, &bs_sectors); > >> - total_sectors2 = bs_sectors; > >> + total_sectors1 = bdrv_nb_sectors(bs1); > >> + if (total_sectors1 < 0) { > >> + error_report("Can't get size of %s: %s", > >> + filename1, strerror(-total_sectors1)); > >> + ret = 4; > >> + goto out; > >> + } > >> + total_sectors2 = bdrv_nb_sectors(bs2); > >> + if (total_sectors2 < 0) { > >> + error_report("Can't get size of %s: %s", > >> + filename2, strerror(-total_sectors2)); > >> + ret = 4; > >> + goto out; > >> + } > >> total_sectors = MIN(total_sectors1, total_sectors2); > >> progress_base = MAX(total_sectors1, total_sectors2); > >> > >> @@ -1185,7 +1194,7 @@ static int img_convert(int argc, char **argv) > >> BlockDriver *drv, *proto_drv; > >> BlockDriverState **bs = NULL, *out_bs = NULL; > >> int64_t total_sectors, nb_sectors, sector_num, bs_offset; > >> - uint64_t *bs_sectors = NULL; > >> + int64_t *bs_sectors = NULL; > >> uint8_t * buf = NULL; > >> size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE; > >> const uint8_t *buf1; > >> @@ -1326,7 +1335,7 @@ static int img_convert(int argc, char **argv) > >> qemu_progress_print(0, 100); > >> > >> bs = g_new0(BlockDriverState *, bs_n); > >> - bs_sectors = g_new(uint64_t, bs_n); > >> + bs_sectors = g_new(int64_t, bs_n); > >> > >> total_sectors = 0; > >> for (bs_i = 0; bs_i < bs_n; bs_i++) { > >> @@ -1340,7 +1349,13 @@ static int img_convert(int argc, char **argv) > >> ret = -1; > >> goto out; > >> } > >> - bdrv_get_geometry(bs[bs_i], &bs_sectors[bs_i]); > >> + bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]); > >> + if (bs_sectors[bs_i] < 0) { > >> + error_report("Could not get size of %s: %s", > >> + argv[optind + bs_i], strerror(-bs_sectors[bs_i])); > >> + ret = -1; > >> + goto out; > >> + } > >> total_sectors += bs_sectors[bs_i]; > >> } > >> > >> @@ -2421,9 +2436,9 @@ static int img_rebase(int argc, char **argv) > >> * the image is the same as the original one at any time. > >> */ > >> if (!unsafe) { > >> - uint64_t num_sectors; > >> - uint64_t old_backing_num_sectors; > >> - uint64_t new_backing_num_sectors = 0; > >> + int64_t num_sectors; > >> + int64_t old_backing_num_sectors; > >> + int64_t new_backing_num_sectors = 0; > >> uint64_t sector; > >> int n; > >> uint8_t * buf_old; > >> @@ -2433,10 +2448,31 @@ static int img_rebase(int argc, char **argv) > >> buf_old = qemu_blockalign(bs, IO_BUF_SIZE); > >> buf_new = qemu_blockalign(bs, IO_BUF_SIZE); > >> > >> - bdrv_get_geometry(bs, &num_sectors); > >> - bdrv_get_geometry(bs_old_backing, &old_backing_num_sectors); > >> + num_sectors = bdrv_nb_sectors(bs); > >> + if (num_sectors < 0) { > >> + error_report("Could not get size of '%s': %s", > >> + filename, strerror(-num_sectors)); > >> + ret = -1; > >> + goto out; > >> + } > >> + old_backing_num_sectors = bdrv_nb_sectors(bs_old_backing); > >> + if (old_backing_num_sectors < 0) { > >> + char backing_name[1024]; > > > > Could you put this on the heap ? > > > > I recently fixed a stack overflow when taking snapshots due to multiple PATH_MAX > > char array in a recursive function. > > > > We don't know how this function will be used later. > > img_rebase() is not a general purpose function, it's a qemu-img command. > Stack use is well below a single page even with my patch. I can't see > how it could possibly become recursive. ok
The Wednesday 04 Jun 2014 à 13:51:51 (+0200), Markus Armbruster wrote : > bdrv_get_geometry() hides errors. Use bdrv_nb_sectors() or > bdrv_getlength() instead where that's obviously inappropriate. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Max Reitz <mreitz@redhat.com> > --- > block.c | 11 ++++++++--- > block/qapi.c | 14 +++++++++---- > qemu-img.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++------------- > 3 files changed, 68 insertions(+), 21 deletions(-) > > diff --git a/block.c b/block.c > index cae9085..b92f05f 100644 > --- a/block.c > +++ b/block.c > @@ -5574,7 +5574,7 @@ void bdrv_img_create(const char *filename, const char *fmt, > if (size && size->value.n == -1) { > if (backing_file && backing_file->value.s) { > BlockDriverState *bs; > - uint64_t size; > + int64_t size; > char buf[32]; > int back_flags; > > @@ -5593,8 +5593,13 @@ void bdrv_img_create(const char *filename, const char *fmt, > local_err = NULL; > goto out; > } > - bdrv_get_geometry(bs, &size); > - size *= 512; > + size = bdrv_getlength(bs); > + if (size < 0) { > + error_setg_errno(errp, -size, "Could not get size of '%s'", > + backing_file->value.s); > + bdrv_unref(bs); > + goto out; > + } > > snprintf(buf, sizeof(buf), "%" PRId64, size); > set_option_parameter(param, BLOCK_OPT_SIZE, buf); > diff --git a/block/qapi.c b/block/qapi.c > index 97e1641..90f406d 100644 > --- a/block/qapi.c > +++ b/block/qapi.c > @@ -165,19 +165,25 @@ void bdrv_query_image_info(BlockDriverState *bs, > ImageInfo **p_info, > Error **errp) > { > - uint64_t total_sectors; > + int64_t size; > const char *backing_filename; > char backing_filename2[1024]; > BlockDriverInfo bdi; > int ret; > Error *err = NULL; > - ImageInfo *info = g_new0(ImageInfo, 1); > + ImageInfo *info; > > - bdrv_get_geometry(bs, &total_sectors); > + size = bdrv_getlength(bs); > + if (size < 0) { > + error_setg_errno(errp, -size, "Can't get size of device '%s'", > + bdrv_get_device_name(bs)); > + return; > + } > > + info = g_new0(ImageInfo, 1); > info->filename = g_strdup(bs->filename); > info->format = g_strdup(bdrv_get_format_name(bs)); > - info->virtual_size = total_sectors * 512; > + info->virtual_size = size; > info->actual_size = bdrv_get_allocated_file_size(bs); > info->has_actual_size = info->actual_size >= 0; > if (bdrv_is_encrypted(bs)) { > diff --git a/qemu-img.c b/qemu-img.c > index e6d0edf..7e6dde0 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -958,7 +958,6 @@ static int img_compare(int argc, char **argv) > int64_t sector_num = 0; > int64_t nb_sectors; > int c, pnum; > - uint64_t bs_sectors; > uint64_t progress_base; > > for (;;) { > @@ -1020,10 +1019,20 @@ static int img_compare(int argc, char **argv) > > buf1 = qemu_blockalign(bs1, IO_BUF_SIZE); > buf2 = qemu_blockalign(bs2, IO_BUF_SIZE); > - bdrv_get_geometry(bs1, &bs_sectors); > - total_sectors1 = bs_sectors; > - bdrv_get_geometry(bs2, &bs_sectors); > - total_sectors2 = bs_sectors; > + total_sectors1 = bdrv_nb_sectors(bs1); > + if (total_sectors1 < 0) { > + error_report("Can't get size of %s: %s", > + filename1, strerror(-total_sectors1)); > + ret = 4; > + goto out; > + } > + total_sectors2 = bdrv_nb_sectors(bs2); > + if (total_sectors2 < 0) { > + error_report("Can't get size of %s: %s", > + filename2, strerror(-total_sectors2)); > + ret = 4; > + goto out; > + } > total_sectors = MIN(total_sectors1, total_sectors2); > progress_base = MAX(total_sectors1, total_sectors2); > > @@ -1185,7 +1194,7 @@ static int img_convert(int argc, char **argv) > BlockDriver *drv, *proto_drv; > BlockDriverState **bs = NULL, *out_bs = NULL; > int64_t total_sectors, nb_sectors, sector_num, bs_offset; > - uint64_t *bs_sectors = NULL; > + int64_t *bs_sectors = NULL; > uint8_t * buf = NULL; > size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE; > const uint8_t *buf1; > @@ -1326,7 +1335,7 @@ static int img_convert(int argc, char **argv) > qemu_progress_print(0, 100); > > bs = g_new0(BlockDriverState *, bs_n); > - bs_sectors = g_new(uint64_t, bs_n); > + bs_sectors = g_new(int64_t, bs_n); > > total_sectors = 0; > for (bs_i = 0; bs_i < bs_n; bs_i++) { > @@ -1340,7 +1349,13 @@ static int img_convert(int argc, char **argv) > ret = -1; > goto out; > } > - bdrv_get_geometry(bs[bs_i], &bs_sectors[bs_i]); > + bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]); > + if (bs_sectors[bs_i] < 0) { > + error_report("Could not get size of %s: %s", > + argv[optind + bs_i], strerror(-bs_sectors[bs_i])); > + ret = -1; > + goto out; > + } > total_sectors += bs_sectors[bs_i]; > } > > @@ -2421,9 +2436,9 @@ static int img_rebase(int argc, char **argv) > * the image is the same as the original one at any time. > */ > if (!unsafe) { > - uint64_t num_sectors; > - uint64_t old_backing_num_sectors; > - uint64_t new_backing_num_sectors = 0; > + int64_t num_sectors; > + int64_t old_backing_num_sectors; > + int64_t new_backing_num_sectors = 0; > uint64_t sector; > int n; > uint8_t * buf_old; > @@ -2433,10 +2448,31 @@ static int img_rebase(int argc, char **argv) > buf_old = qemu_blockalign(bs, IO_BUF_SIZE); > buf_new = qemu_blockalign(bs, IO_BUF_SIZE); > > - bdrv_get_geometry(bs, &num_sectors); > - bdrv_get_geometry(bs_old_backing, &old_backing_num_sectors); > + num_sectors = bdrv_nb_sectors(bs); > + if (num_sectors < 0) { > + error_report("Could not get size of '%s': %s", > + filename, strerror(-num_sectors)); > + ret = -1; > + goto out; > + } > + old_backing_num_sectors = bdrv_nb_sectors(bs_old_backing); > + if (old_backing_num_sectors < 0) { > + char backing_name[1024]; > + > + bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name)); > + error_report("Could not get size of '%s': %s", > + backing_name, strerror(-old_backing_num_sectors)); > + ret = -1; > + goto out; > + } > if (bs_new_backing) { > - bdrv_get_geometry(bs_new_backing, &new_backing_num_sectors); > + new_backing_num_sectors = bdrv_nb_sectors(bs_new_backing); > + if (new_backing_num_sectors < 0) { > + error_report("Could not get size of '%s': %s", > + out_baseimg, strerror(-new_backing_num_sectors)); > + ret = -1; > + goto out; > + } > } > > if (num_sectors != 0) { > -- > 1.9.3 > Reviewed-by: Benoit Canet <benoit@irqsave.net>
diff --git a/block.c b/block.c index cae9085..b92f05f 100644 --- a/block.c +++ b/block.c @@ -5574,7 +5574,7 @@ void bdrv_img_create(const char *filename, const char *fmt, if (size && size->value.n == -1) { if (backing_file && backing_file->value.s) { BlockDriverState *bs; - uint64_t size; + int64_t size; char buf[32]; int back_flags; @@ -5593,8 +5593,13 @@ void bdrv_img_create(const char *filename, const char *fmt, local_err = NULL; goto out; } - bdrv_get_geometry(bs, &size); - size *= 512; + size = bdrv_getlength(bs); + if (size < 0) { + error_setg_errno(errp, -size, "Could not get size of '%s'", + backing_file->value.s); + bdrv_unref(bs); + goto out; + } snprintf(buf, sizeof(buf), "%" PRId64, size); set_option_parameter(param, BLOCK_OPT_SIZE, buf); diff --git a/block/qapi.c b/block/qapi.c index 97e1641..90f406d 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -165,19 +165,25 @@ void bdrv_query_image_info(BlockDriverState *bs, ImageInfo **p_info, Error **errp) { - uint64_t total_sectors; + int64_t size; const char *backing_filename; char backing_filename2[1024]; BlockDriverInfo bdi; int ret; Error *err = NULL; - ImageInfo *info = g_new0(ImageInfo, 1); + ImageInfo *info; - bdrv_get_geometry(bs, &total_sectors); + size = bdrv_getlength(bs); + if (size < 0) { + error_setg_errno(errp, -size, "Can't get size of device '%s'", + bdrv_get_device_name(bs)); + return; + } + info = g_new0(ImageInfo, 1); info->filename = g_strdup(bs->filename); info->format = g_strdup(bdrv_get_format_name(bs)); - info->virtual_size = total_sectors * 512; + info->virtual_size = size; info->actual_size = bdrv_get_allocated_file_size(bs); info->has_actual_size = info->actual_size >= 0; if (bdrv_is_encrypted(bs)) { diff --git a/qemu-img.c b/qemu-img.c index e6d0edf..7e6dde0 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -958,7 +958,6 @@ static int img_compare(int argc, char **argv) int64_t sector_num = 0; int64_t nb_sectors; int c, pnum; - uint64_t bs_sectors; uint64_t progress_base; for (;;) { @@ -1020,10 +1019,20 @@ static int img_compare(int argc, char **argv) buf1 = qemu_blockalign(bs1, IO_BUF_SIZE); buf2 = qemu_blockalign(bs2, IO_BUF_SIZE); - bdrv_get_geometry(bs1, &bs_sectors); - total_sectors1 = bs_sectors; - bdrv_get_geometry(bs2, &bs_sectors); - total_sectors2 = bs_sectors; + total_sectors1 = bdrv_nb_sectors(bs1); + if (total_sectors1 < 0) { + error_report("Can't get size of %s: %s", + filename1, strerror(-total_sectors1)); + ret = 4; + goto out; + } + total_sectors2 = bdrv_nb_sectors(bs2); + if (total_sectors2 < 0) { + error_report("Can't get size of %s: %s", + filename2, strerror(-total_sectors2)); + ret = 4; + goto out; + } total_sectors = MIN(total_sectors1, total_sectors2); progress_base = MAX(total_sectors1, total_sectors2); @@ -1185,7 +1194,7 @@ static int img_convert(int argc, char **argv) BlockDriver *drv, *proto_drv; BlockDriverState **bs = NULL, *out_bs = NULL; int64_t total_sectors, nb_sectors, sector_num, bs_offset; - uint64_t *bs_sectors = NULL; + int64_t *bs_sectors = NULL; uint8_t * buf = NULL; size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE; const uint8_t *buf1; @@ -1326,7 +1335,7 @@ static int img_convert(int argc, char **argv) qemu_progress_print(0, 100); bs = g_new0(BlockDriverState *, bs_n); - bs_sectors = g_new(uint64_t, bs_n); + bs_sectors = g_new(int64_t, bs_n); total_sectors = 0; for (bs_i = 0; bs_i < bs_n; bs_i++) { @@ -1340,7 +1349,13 @@ static int img_convert(int argc, char **argv) ret = -1; goto out; } - bdrv_get_geometry(bs[bs_i], &bs_sectors[bs_i]); + bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]); + if (bs_sectors[bs_i] < 0) { + error_report("Could not get size of %s: %s", + argv[optind + bs_i], strerror(-bs_sectors[bs_i])); + ret = -1; + goto out; + } total_sectors += bs_sectors[bs_i]; } @@ -2421,9 +2436,9 @@ static int img_rebase(int argc, char **argv) * the image is the same as the original one at any time. */ if (!unsafe) { - uint64_t num_sectors; - uint64_t old_backing_num_sectors; - uint64_t new_backing_num_sectors = 0; + int64_t num_sectors; + int64_t old_backing_num_sectors; + int64_t new_backing_num_sectors = 0; uint64_t sector; int n; uint8_t * buf_old; @@ -2433,10 +2448,31 @@ static int img_rebase(int argc, char **argv) buf_old = qemu_blockalign(bs, IO_BUF_SIZE); buf_new = qemu_blockalign(bs, IO_BUF_SIZE); - bdrv_get_geometry(bs, &num_sectors); - bdrv_get_geometry(bs_old_backing, &old_backing_num_sectors); + num_sectors = bdrv_nb_sectors(bs); + if (num_sectors < 0) { + error_report("Could not get size of '%s': %s", + filename, strerror(-num_sectors)); + ret = -1; + goto out; + } + old_backing_num_sectors = bdrv_nb_sectors(bs_old_backing); + if (old_backing_num_sectors < 0) { + char backing_name[1024]; + + bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name)); + error_report("Could not get size of '%s': %s", + backing_name, strerror(-old_backing_num_sectors)); + ret = -1; + goto out; + } if (bs_new_backing) { - bdrv_get_geometry(bs_new_backing, &new_backing_num_sectors); + new_backing_num_sectors = bdrv_nb_sectors(bs_new_backing); + if (new_backing_num_sectors < 0) { + error_report("Could not get size of '%s': %s", + out_baseimg, strerror(-new_backing_num_sectors)); + ret = -1; + goto out; + } } if (num_sectors != 0) {