From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42609) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WQzZP-0003Aw-NO for qemu-devel@nongnu.org; Fri, 21 Mar 2014 09:33:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WQzZL-0001hq-Au for qemu-devel@nongnu.org; Fri, 21 Mar 2014 09:33:11 -0400 Received: from mail-oa0-f43.google.com ([209.85.219.43]:47620) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WQzZL-0001hg-55 for qemu-devel@nongnu.org; Fri, 21 Mar 2014 09:33:07 -0400 Received: by mail-oa0-f43.google.com with SMTP id eb12so2515711oac.30 for ; Fri, 21 Mar 2014 06:33:06 -0700 (PDT) Date: Fri, 21 Mar 2014 13:31:42 +0000 From: Leandro Dorileo Message-ID: <20140321133142.GA22259@dorilex> References: <1395360813-2833-1-git-send-email-l@dorileo.org> <1395360813-2833-10-git-send-email-l@dorileo.org> <532BDFA0.7050807@kamp.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <532BDFA0.7050807@kamp.de> 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: Peter Lieven 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 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. > Peter > > bs->opaque = g_malloc0(sizeof(struct IscsiLun)); > >@@ -1451,13 +1447,17 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) > > return 0; > > } > >-static QEMUOptionParameter iscsi_create_options[] = { > >- { > >- .name = BLOCK_OPT_SIZE, > >- .type = OPT_SIZE, > >- .help = "Virtual disk size" > >+static QemuOptsList iscsi_create_options = { > >+ .name = "iscsi_create_options", > >+ .head = QTAILQ_HEAD_INITIALIZER(iscsi_create_options.head), > >+ .desc = { > >+ { > >+ .name = BLOCK_OPT_SIZE, > >+ .type = QEMU_OPT_SIZE, > >+ .help = "Virtual disk size" > >+ }, > >+ { NULL } > > }, > >- { NULL } > > }; > > static BlockDriver bdrv_iscsi = { > >@@ -1469,7 +1469,7 @@ static BlockDriver bdrv_iscsi = { > > .bdrv_file_open = iscsi_open, > > .bdrv_close = iscsi_close, > > .bdrv_create = iscsi_create, > >- .create_options = iscsi_create_options, > >+ .create_options = &iscsi_create_options, > > .bdrv_reopen_prepare = iscsi_reopen_prepare, > > .bdrv_getlength = iscsi_getlength, > > > -- > > Mit freundlichen Grüßen > > Peter Lieven > > ........................................................... > > KAMP Netzwerkdienste GmbH > Vestische Str. 89-91 | 46117 Oberhausen > Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40 > pl@kamp.de | http://www.kamp.de > > Geschäftsführer: Heiner Lante | Michael Lante > Amtsgericht Duisburg | HRB Nr. 12154 > USt-Id-Nr.: DE 120607556 > > ........................................................... > > -- Leandro Dorileo