qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 09/26] iscsi: migrate iscsi driver QemuOptionPar


From: Peter Lieven
Subject: Re: [Qemu-devel] [PATCH 09/26] iscsi: migrate iscsi driver QemuOptionParameter usage
Date: Fri, 21 Mar 2014 14:42:35 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0

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 <address@hidden>
---
  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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]