From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51439) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WrU49-0003BJ-Qy for qemu-devel@nongnu.org; Mon, 02 Jun 2014 11:22:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WrU42-000110-BK for qemu-devel@nongnu.org; Mon, 02 Jun 2014 11:22:25 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:38647 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WrU42-00010u-39 for qemu-devel@nongnu.org; Mon, 02 Jun 2014 11:22:18 -0400 Date: Mon, 2 Jun 2014 17:22:17 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140602152216.GJ8181@irqsave.net> References: <1401473631-10724-1-git-send-email-armbru@redhat.com> <1401473631-10724-11-git-send-email-armbru@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1401473631-10724-11-git-send-email-armbru@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 10/10] block: Avoid bdrv_get_geometry() where errors should be detected List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mreitz@redhat.com The Friday 30 May 2014 =E0 20:13:51 (+0200), Markus Armbruster wrote : > bdrv_get_geometry() hides errors. Use bdrv_nb_sectors() or > bdrv_getlength() instead where that's obviously inappropriate. >=20 > Signed-off-by: Markus Armbruster > Reviewed-by: Eric Blake > Reviewed-by: Max Reitz > --- > block.c | 11 ++++++++--- > block/qapi.c | 14 ++++++++++---- > qemu-img.c | 59 +++++++++++++++++++++++++++++++++++++++++++++-------= ------- > 3 files changed, 63 insertions(+), 21 deletions(-) >=20 > diff --git a/block.c b/block.c > index 41837b4..94f8b87 100644 > --- a/block.c > +++ b/block.c > @@ -5573,7 +5573,7 @@ void bdrv_img_create(const char *filename, const = char *fmt, > if (size && size->value.n =3D=3D -1) { > if (backing_file && backing_file->value.s) { > BlockDriverState *bs; > - uint64_t size; > + int64_t size; > char buf[32]; > int back_flags; > =20 > @@ -5592,8 +5592,13 @@ void bdrv_img_create(const char *filename, const= char *fmt, > local_err =3D NULL; > goto out; > } > - bdrv_get_geometry(bs, &size); > - size *=3D 512; > + size =3D 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; > + } > =20 > 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 75f44f1..ae6488a 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 =3D NULL; > - ImageInfo *info =3D g_new0(ImageInfo, 1); > + ImageInfo *info; > =20 > - bdrv_get_geometry(bs, &total_sectors); > + size =3D bdrv_getlength(bs); > + if (size < 0) { > + error_setg_errno(errp, -size, "Can't get size of device '%s'", > + bdrv_get_device_name(bs)); > + return; > + } > =20 > + info =3D g_new0(ImageInfo, 1); > info->filename =3D g_strdup(bs->filename); > info->format =3D g_strdup(bdrv_get_format_name(bs)); > - info->virtual_size =3D total_sectors * 512; > + info->virtual_size =3D size; > info->actual_size =3D bdrv_get_allocated_file_size(bs); > info->has_actual_size =3D info->actual_size >=3D 0; > if (bdrv_is_encrypted(bs)) { > diff --git a/qemu-img.c b/qemu-img.c > index 2cb56c5..fca58bc 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -959,7 +959,6 @@ static int img_compare(int argc, char **argv) > int64_t sector_num =3D 0; > int64_t nb_sectors; > int c, pnum; > - uint64_t bs_sectors; > uint64_t progress_base; > =20 > for (;;) { > @@ -1021,10 +1020,18 @@ static int img_compare(int argc, char **argv) > =20 > buf1 =3D qemu_blockalign(bs1, IO_BUF_SIZE); > buf2 =3D qemu_blockalign(bs2, IO_BUF_SIZE); > - bdrv_get_geometry(bs1, &bs_sectors); > - total_sectors1 =3D bs_sectors; > - bdrv_get_geometry(bs2, &bs_sectors); > - total_sectors2 =3D bs_sectors; > + total_sectors1 =3D bdrv_nb_sectors(bs1); > + if (total_sectors1 < 0) { > + error_report("Can't get size of %s: %s", > + filename1, strerror(-total_sectors1)); > + goto out; > + } > + total_sectors2 =3D bdrv_nb_sectors(bs2); > + if (total_sectors2 < 0) { > + error_report("Can't get size of %s: %s", > + filename2, strerror(-total_sectors2)); > + goto out; > + } Would setting ret be useful beforie goto out ?=20 > total_sectors =3D MIN(total_sectors1, total_sectors2); > progress_base =3D MAX(total_sectors1, total_sectors2); > =20 OA> @@ -1186,7 +1193,7 @@ static int img_convert(int argc, char **argv) > BlockDriver *drv, *proto_drv; > BlockDriverState **bs =3D NULL, *out_bs =3D NULL; > int64_t total_sectors, nb_sectors, sector_num, bs_offset; > - uint64_t *bs_sectors =3D NULL; > + int64_t *bs_sectors =3D NULL; > uint8_t * buf =3D NULL; > size_t bufsectors =3D IO_BUF_SIZE / BDRV_SECTOR_SIZE; > const uint8_t *buf1; > @@ -1327,7 +1334,7 @@ static int img_convert(int argc, char **argv) > qemu_progress_print(0, 100); > =20 > bs =3D g_new0(BlockDriverState *, bs_n); > - bs_sectors =3D g_new(uint64_t, bs_n); > + bs_sectors =3D g_new(int64_t, bs_n); > =20 > total_sectors =3D 0; > for (bs_i =3D 0; bs_i < bs_n; bs_i++) { > @@ -1341,7 +1348,13 @@ static int img_convert(int argc, char **argv) > ret =3D -1; > goto out; > } > - bdrv_get_geometry(bs[bs_i], &bs_sectors[bs_i]); > + bs_sectors[bs_i] =3D 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 =3D -1; > + goto out; > + } > total_sectors +=3D bs_sectors[bs_i]; > } > =20 > @@ -2422,9 +2435,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 =3D 0; > + int64_t num_sectors; > + int64_t old_backing_num_sectors; > + int64_t new_backing_num_sectors =3D 0; > uint64_t sector; > int n; > uint8_t * buf_old; > @@ -2434,10 +2447,28 @@ static int img_rebase(int argc, char **argv) > buf_old =3D qemu_blockalign(bs, IO_BUF_SIZE); > buf_new =3D qemu_blockalign(bs, IO_BUF_SIZE); > =20 > - bdrv_get_geometry(bs, &num_sectors); > - bdrv_get_geometry(bs_old_backing, &old_backing_num_sectors); > + num_sectors =3D bdrv_nb_sectors(bs); > + if (num_sectors < 0) { > + error_report("Could not get size of '%s': %s", > + filename, strerror(-num_sectors)); > + goto out; > + } > + old_backing_num_sectors =3D 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_secto= rs)); > + goto out; > + } > if (bs_new_backing) { > - bdrv_get_geometry(bs_new_backing, &new_backing_num_sectors= ); > + new_backing_num_sectors =3D 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_se= ctors)); > + goto out; > + } > } > =20 > if (num_sectors !=3D 0) { > --=20 > 1.9.3 >=20 >=20