From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49998) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDjv4-0003eY-HW for qemu-devel@nongnu.org; Wed, 12 Feb 2014 19:12:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WDjv0-0007sr-4K for qemu-devel@nongnu.org; Wed, 12 Feb 2014 19:12:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:8386) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDjuz-0007sb-RZ for qemu-devel@nongnu.org; Wed, 12 Feb 2014 19:12:42 -0500 Message-ID: <52FC0DF5.4030307@redhat.com> Date: Wed, 12 Feb 2014 17:12:37 -0700 From: Eric Blake MIME-Version: 1.0 References: <1392186806-10418-1-git-send-email-cyliu@suse.com> <1392186806-10418-7-git-send-email-cyliu@suse.com> In-Reply-To: <1392186806-10418-7-git-send-email-cyliu@suse.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Hpp72XcAdC6DHhl8LJ5FX4DXVusUB6iDG" Subject: Re: [Qemu-devel] [PATCH v20 06/26] change block layer to support both QemuOpts and QEMUOptionParameter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chunyan Liu , qemu-devel@nongnu.org Cc: kwolf@redhat.com, Dong Xu Wang , stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Hpp72XcAdC6DHhl8LJ5FX4DXVusUB6iDG Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 02/11/2014 11:33 PM, Chunyan Liu wrote: > Change block layer to support both QemuOpts and QEMUOptionParameter. > After this patch, it will change backend drivers one by one. At the end= , > QEMUOptionParameter will be removed and only QemuOpts is kept. >=20 > Signed-off-by: Dong Xu Wang > Signed-off-by: Chunyan Liu > --- > block.c | 110 ++++++++++++++++++++++++++++---------= ------- > block/cow.c | 2 +- > block/qcow.c | 2 +- > block/qcow2.c | 2 +- > block/qed.c | 2 +- > block/raw_bsd.c | 2 +- > block/vhdx.c | 2 +- > block/vmdk.c | 4 +- > block/vvfat.c | 2 +- > include/block/block.h | 4 +- > include/block/block_int.h | 4 +- > include/qemu/option.h | 2 + > qemu-img.c | 87 +++++++++++++++++++++-------------- > util/qemu-option.c | 111 +++++++++++++++++++++++++++++++++++++= ++++++++ > 14 files changed, 250 insertions(+), 86 deletions(-) >=20 > +++ b/include/block/block_int.h > @@ -118,6 +118,8 @@ struct BlockDriver { > void (*bdrv_rebind)(BlockDriverState *bs); > int (*bdrv_create)(const char *filename, QEMUOptionParameter *opti= ons, > Error **errp); > + int (*bdrv_create2)(const char *filename, QemuOpts *opts, > + Error **errp); Maybe a FIXME comment that shows we plan on removing the duplicate and renaming back to a single sane name in a few more commits. > +++ b/include/qemu/option.h > @@ -168,4 +168,6 @@ int qemu_opts_foreach(QemuOptsList *list, qemu_opts= _loopfunc func, void *opaque, > QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);= > void qemu_opts_free(QemuOptsList *list); > void qemu_opts_print_help(QemuOptsList *list); > +QEMUOptionParameter *opts_to_params(QemuOpts *opts); > +QemuOptsList *params_to_opts(QEMUOptionParameter *list); I'd split this commit into two - one that adds these two conversion functions, then the other that adds the create2 callback. > +++ b/util/qemu-option.c > @@ -1396,3 +1396,114 @@ void qemu_opts_print_help(QemuOptsList *list) > list->desc[i].help : ""); > } > } > + > +/* convert QEMUOptionParameter to QemuOpts */ > +QemuOptsList *params_to_opts(QEMUOptionParameter *list) > +{ > + QemuOptsList *opts =3D NULL; > + size_t num_opts, i =3D 0; > + > + if (!list) { > + return NULL; > + } > + > + num_opts =3D count_option_parameters(list); > + opts =3D g_malloc0(sizeof(QemuOptsList) + > + (num_opts + 1) * sizeof(QemuOptDesc)); Indentation looks off. > + QTAILQ_INIT(&opts->head); > + opts->desc[i].name =3D NULL; > + > + while (list && list->name) { > + opts->desc[i].name =3D strdup(list->name); > + opts->desc[i].help =3D strdup(list->help); > + switch (list->type) { > + case OPT_FLAG: > + opts->desc[i].type =3D QEMU_OPT_BOOL; > + opts->desc[i].def_value_str =3D list->value.n ? "on" : "of= f"; > + break; > + > + case OPT_NUMBER: > + opts->desc[i].type =3D QEMU_OPT_NUMBER; > + if (list->value.n) { > + char tmp[100]; > + sprintf(tmp, "%" PRIu64, list->value.n); > + opts->desc[i].def_value_str =3D strdup(tmp); Eww. Just use g_strdup_printf and avoid tmp[] altogether. > + } > + break; > + > + case OPT_SIZE: > + opts->desc[i].type =3D QEMU_OPT_SIZE; > + if (list->value.n) { > + char tmp[100]; > + sprintf(tmp, "%" PRIu64, list->value.n); > + opts->desc[i].def_value_str =3D strdup(tmp); and again > + } > + break; > + > + case OPT_STRING: > + opts->desc[i].type =3D QEMU_OPT_STRING; > + if (list->value.s) { > + opts->desc[i].def_value_str =3D strdup(list->value.s);= This is a lot of use of strdup() without checking for malloc failure; wouldn't it be better to use g_strdup (which can't fail, and which gracefully handles NULL so you can also drop the 'if') and fixing the counterpart free to use g_free? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Hpp72XcAdC6DHhl8LJ5FX4DXVusUB6iDG Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJS/A31AAoJEKeha0olJ0Nqj2kH/33uHTPrglhdPT3Q7Wisod64 8RrfVBCOBKzAzjzrmYJGi1sbNJaqw1eCLlUm3ftJ45JVoV41VhY1fkq50krwOFe9 f0XpIGf21WGaMwbrc+TfEMRVu6cFIZVcrRi97kxZ9eszGMIM9Iqs6rmcgHBHrEGs RbWv7emhclSJZApmh5NCCdUmaImlLE7+ll1HEdy7BYjjPWK+rY0ug5B2JyrEjJ1C ctlr8JuZ9OTz95C+UbhKkWMvu2cY9Ez+ed3lYkVQz9oqv5a/ly9GRz7UagZ3Xm7j duzxMYo3RLR8B2Wo5OrIzpntCrU+52bd7aNG5190JlI7BxzrSJGvJZVh83dzsr8= =x10f -----END PGP SIGNATURE----- --Hpp72XcAdC6DHhl8LJ5FX4DXVusUB6iDG--