From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44487) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WQzis-0007TD-97 for qemu-devel@nongnu.org; Fri, 21 Mar 2014 09:43:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WQzim-0004bu-ML for qemu-devel@nongnu.org; Fri, 21 Mar 2014 09:42:58 -0400 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:37941 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WQzim-0004bi-Bz for qemu-devel@nongnu.org; Fri, 21 Mar 2014 09:42:52 -0400 Message-ID: <532C41CB.5080306@kamp.de> Date: Fri, 21 Mar 2014 14:42:35 +0100 From: Peter Lieven MIME-Version: 1.0 References: <1395360813-2833-1-git-send-email-l@dorileo.org> <1395360813-2833-10-git-send-email-l@dorileo.org> <532BDFA0.7050807@kamp.de> <20140321133142.GA22259@dorilex> In-Reply-To: <20140321133142.GA22259@dorilex> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 09/26] iscsi: migrate iscsi driver QemuOptionParameter usage List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leandro Dorileo Cc: Kevin Wolf , Fam Zheng , Ronnie Sahlberg , "Richard W.M. Jones" , Stefan Weil , Jeff Cody , qemu-devel@nongnu.org, Markus Armbruster , Paolo Bonzini , Stefan Hajnoczi , Josh Durgin , Anthony Liguori , Liu Yuan , Luiz Capitulino , Max Reitz , MORITA Kazutaka , Benoit Canet On 21.03.2014 14:31, Leandro Dorileo wrote: > On Fri, Mar 21, 2014 at 07:43:44AM +0100, Peter Lieven wrote: >> On 21.03.2014 01:13, Leandro Dorileo wrote: >>> Do the directly migration from QemuOptionParameter to QemuOpts on >>> iscsi block driver. >>> >>> Signed-off-by: Leandro Dorileo >>> --- >>> block/iscsi.c | 32 ++++++++++++++++---------------- >>> 1 file changed, 16 insertions(+), 16 deletions(-) >>> >>> diff --git a/block/iscsi.c b/block/iscsi.c >>> index b490e98..85252e7 100644 >>> --- a/block/iscsi.c >>> +++ b/block/iscsi.c >>> @@ -1125,7 +1125,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, >>> QemuOpts *opts; >>> Error *local_err = NULL; >>> const char *filename; >>> - int i, ret; >>> + int i, ret = 0; >> why? is there a chance that ret remains uninitialized? > Yep, my compiler tells me so: > > block/iscsi.c:1128:12: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > > >>> if ((BDRV_SECTOR_SIZE % 512) != 0) { >>> error_setg(errp, "iSCSI: Invalid BDRV_SECTOR_SIZE. " >>> @@ -1382,8 +1382,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset) >>> return 0; >>> } >>> -static int iscsi_create(const char *filename, QEMUOptionParameter *options, >>> - Error **errp) >>> +static int iscsi_create(const char *filename, QemuOpts *options, Error **errp) >>> { >>> int ret = 0; >>> int64_t total_size = 0; >>> @@ -1393,12 +1392,9 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options, >>> bs = bdrv_new(""); >>> - /* Read out options */ >>> - while (options && options->name) { >>> - if (!strcmp(options->name, "size")) { >>> - total_size = options->value.n / BDRV_SECTOR_SIZE; >>> - } >>> - options++; >>> + total_size = qemu_opt_get_size(options, BLOCK_OPT_SIZE, 0); >>> + if (total_size) { >>> + total_size = total_size / BDRV_SECTOR_SIZE; >>> } >> you don't need the if condition. 0 / BDRV_SECTOR_SIZE = 0. >> > I'm not sure, bdrv_img_create() will set BLOCK_OPT_SIZE with img_size, we have no guarantee on the > value passed to bdrv_img_create(), we don't check img_size value there, having said that can't > we run on division by zero here? The previous code wasn't checking it but I wonder if the problem > wasn't there already. division by zero is x / 0 not 0 / x. 0 / x = 0 x / 0 = undef Peter