From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43193) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WQzbg-0004eh-Tz for qemu-devel@nongnu.org; Fri, 21 Mar 2014 09:35:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WQzbb-0002b4-6m for qemu-devel@nongnu.org; Fri, 21 Mar 2014 09:35:32 -0400 Received: from mail-oa0-f53.google.com ([209.85.219.53]:34853) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WQzbb-0002az-0B for qemu-devel@nongnu.org; Fri, 21 Mar 2014 09:35:27 -0400 Received: by mail-oa0-f53.google.com with SMTP id j17so2510396oag.40 for ; Fri, 21 Mar 2014 06:35:26 -0700 (PDT) Date: Fri, 21 Mar 2014 13:34:11 +0000 From: Leandro Dorileo Message-ID: <20140321133411.GB22259@dorilex> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20140321133142.GA22259@dorilex> 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 01:31:42PM +0000, 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. Ok, qemu-img does guarantee the img_size value... > > > > 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 -- Leandro Dorileo