From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:49511)
	by lists.gnu.org with esmtp (Exim 4.71) (envelope-from <pl@kamp.de>)
	id 1VoEoT-00020A-F8
	for qemu-devel@nongnu.org; Wed, 04 Dec 2013 10:56:43 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pl@kamp.de>) id 1VoEoJ-00045I-Uw
	for qemu-devel@nongnu.org; Wed, 04 Dec 2013 10:56:33 -0500
Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:50702 helo=mx01.kamp.de)
	by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <pl@kamp.de>)
	id 1VoEoJ-00043x-Kc
	for qemu-devel@nongnu.org; Wed, 04 Dec 2013 10:56:23 -0500
Message-ID: <529F50A3.3090106@kamp.de>
Date: Wed, 04 Dec 2013 16:56:19 +0100
From: Peter Lieven <pl@kamp.de>
MIME-Version: 1.0
References: <1385546829-3839-1-git-send-email-pl@kamp.de>
	<1385546829-3839-8-git-send-email-pl@kamp.de>
	<20131204154904.GD27759@stefanha-thinkpad.redhat.com>
In-Reply-To: <20131204154904.GD27759@stefanha-thinkpad.redhat.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCHv3 1.8 7/9] qemu-img: round down request
 length to an aligned sector
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com

Am 04.12.2013 16:49, schrieb Stefan Hajnoczi:
> On Wed, Nov 27, 2013 at 11:07:07AM +0100, Peter Lieven wrote:
>> @@ -1397,19 +1396,21 @@ static int img_convert(int argc, char **argv)
>>          }
>>      }
>>  
>> +    cluster_sectors = 0;
>> +    ret = bdrv_get_info(out_bs, &bdi);
>> +    if (ret < 0 && compress) {
>> +        error_report("could not get block driver info");
>> +        goto out;
>> +    } else {
>> +        cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
>> +    }
> Why do we only report error if 'compress' is set?  cluster_sectors must
> be valid and we cannot guarantee that if bdrv_get_info() failed.
You mean this should be:

+    if (ret < 0) {
+        if (compress) {
+            error_report("could not get block driver info");
+            goto out;
+        }
+    } else {
+        cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
+    }


if cluster_sectors is 0 the alignment logic is skipped, but we cannot
guarantee that bdi is zero and stays zero if the call fails.

can you fix that when you pick up the patch?

Peter