From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57982) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFetx-0000yY-2D for qemu-devel@nongnu.org; Tue, 18 Feb 2014 02:15:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFetv-0006YW-Jz for qemu-devel@nongnu.org; Tue, 18 Feb 2014 02:15:32 -0500 Received: from mail-vc0-x229.google.com ([2607:f8b0:400c:c03::229]:42638) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFetv-0006YO-5z for qemu-devel@nongnu.org; Tue, 18 Feb 2014 02:15:31 -0500 Received: by mail-vc0-f169.google.com with SMTP id hq11so13002489vcb.0 for ; Mon, 17 Feb 2014 23:15:30 -0800 (PST) MIME-Version: 1.0 Sender: kgrace.liu@gmail.com In-Reply-To: <52FC0F7A.3000901@redhat.com> References: <1392186806-10418-1-git-send-email-cyliu@suse.com> <1392186806-10418-8-git-send-email-cyliu@suse.com> <52FC0F7A.3000901@redhat.com> Date: Tue, 18 Feb 2014 15:15:30 +0800 Message-ID: From: Chunyan Liu Content-Type: multipart/alternative; boundary=089e0158a82a91850404f2a90913 Subject: Re: [Qemu-devel] [PATCH v20 07/26] cow.c: replace QEMUOptionParameter with QemuOpts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Kevin Wolf , Dong Xu Wang , qemu-devel@nongnu.org, stefanha@redhat.com --089e0158a82a91850404f2a90913 Content-Type: text/plain; charset=UTF-8 2014-02-13 8:19 GMT+08:00 Eric Blake : > On 02/11/2014 11:33 PM, Chunyan Liu wrote: > > cow.c: replace QEMUOptionParameter with QemuOpts > > > > Signed-off-by: Dong Xu Wang > > Signed-off-by: Chunyan Liu > > --- > > block/cow.c | 46 ++++++++++++++++++++++------------------------ > > 1 files changed, 22 insertions(+), 24 deletions(-) > > > > > +static QemuOptsList cow_create_opts = { > > + .name = "cow-create-opts", > > + .head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head), > > + .desc = { > > + { > > + .name = BLOCK_OPT_SIZE, > > + .type = QEMU_OPT_SIZE, > > + .help = "Virtual disk size" > > Oh, these QemuOpts really ARE constant strings in the general case, and > passing these strings to free() could be a disaster. I hope we're okay > in what we do; in fact, maybe it's worth a patch to QemuOptsList that > adds a bool record of whether the list is dynamically allocated (false > by default for all static initialization, and true when malloc'ing a > list) and then asserting that attempts to free (parts of) a list are > only done on a dynamically allocated list. > > Well, create_opts specified here won't be freed by free() in existing code I thi nk. Those using it would malloc and memcpy from it, and that would be freed. The same way as existing create_options handling. > > @@ -414,14 +412,14 @@ static BlockDriver bdrv_cow = { > > .bdrv_probe = cow_probe, > > .bdrv_open = cow_open, > > .bdrv_close = cow_close, > > - .bdrv_create = cow_create, > > + .bdrv_create2 = cow_create, > > Might as well keep alignment looking nice. > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > > --089e0158a82a91850404f2a90913 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable



2014-02-13 8:19 GMT+08:00 Eric Blake <eblake@redhat.com>:
On 02/11/= 2014 11:33 PM, Chunyan Liu wrote:
> cow.c: replace QEMUOptionParameter with QemuOpts
>
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> Signed-off-by: Chunyan Liu <cyliu= @suse.com>
> ---
> =C2=A0block/cow.c | =C2=A0 46 ++++++++++++++++++++++------------------= ------
> =C2=A01 files changed, 22 insertions(+), 24 deletions(-)
>

> +static QemuOptsList cow_create_opts =3D {
> + =C2=A0 =C2=A0.name =3D "cow-create-opts",
> + =C2=A0 =C2=A0.head =3D QTAILQ_HEAD_INITIALIZER(cow_create_opts.head)= ,
> + =C2=A0 =C2=A0.desc =3D {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0{
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.name =3D BLOCK_OPT_SIZE, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.type =3D QEMU_OPT_SIZE, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.help =3D "Virtual dis= k size"

Oh, these QemuOpts really ARE constant strings in the general case, a= nd
passing these strings to free() could be a disaster. =C2=A0I hope we're= okay
in what we do; in fact, maybe it's worth a patch to QemuOptsList that adds a bool record of whether the list is dynamically allocated (false
by default for all static initialization, and true when malloc'ing a list) and then asserting that attempts to free (parts of) a list are
only done on a dynamically allocated list.

Well, create_opts specified her= e won't be freed by free() in existing code I thi
nk. Those using it= would malloc and memcpy from it, and that would be freed. The
same way = as existing create_options handling.
=C2=A0
> @@ -414,14 +412,14 @@ static BlockDriver bdrv_cow =3D {
> =C2=A0 =C2=A0 =C2=A0.bdrv_probe =C2=A0 =C2=A0 =3D cow_probe,
> =C2=A0 =C2=A0 =C2=A0.bdrv_open =C2=A0 =C2=A0 =C2=A0=3D cow_open,
> =C2=A0 =C2=A0 =C2=A0.bdrv_close =C2=A0 =C2=A0 =3D cow_close,
> - =C2=A0 =C2=A0.bdrv_create =C2=A0 =C2=A0=3D cow_create,
> + =C2=A0 =C2=A0.bdrv_create2 =C2=A0 =C2=A0=3D cow_create,

Might as well keep alignment looking nice.

--
Eric Blake =C2=A0 eblake redhat com =C2=A0 =C2=A0+1-919-301-3266
Libvirt virtualization library http://libvirt.org


--089e0158a82a91850404f2a90913--