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 <l...@dorileo.org>
> > >---
> > > 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
> > p...@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
