On Wed, Dec 04, 2013 at 04:56:19PM +0100, Peter Lieven wrote: > 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?
Sure. Stefan