Re: [Qemu-devel] [PATCH v20 17/26] rbd.c: replace QEMUOptionParameter with QemuOpts

[Chunyan Liu <cyliu@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

2014-02-18 2:57 GMT+08:00 Eric Blake <eblake@redhat.com>:

> On 02/11/2014 11:33 PM, Chunyan Liu wrote:
> > rbd.c: replace QEMUOptionParameter with QemuOpts
> >
> > Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> > Signed-off-by: Chunyan Liu <cyliu@suse.com>
> > ---
> >  block/rbd.c |   63
> +++++++++++++++++++++++++++++------------------------------
> >  1 files changed, 31 insertions(+), 32 deletions(-)
> >
>
> > +    objsize = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE, 0);
> > +    if (objsize) {
> > +        if ((objsize - 1) & objsize) {    /* not a power of 2? */
> > +            error_report("obj size needs to be power of 2");
> > +            return -EINVAL;
> > +        }
> > +        if (objsize < 4096) {
> > +            error_report("obj size too small");
> > +            return -EINVAL;
> >          }
>
> > +        {
> > +            .name = BLOCK_OPT_CLUSTER_SIZE,
> > +            .type = QEMU_OPT_SIZE,
> > +            .help = "RBD object size",
> > +            .def_value_str = stringify(0),
>
> Do we really want to list a default of 0, given that the code behaves
> the same as if 0 had been specified or if the argument had been omitted?
>  Passing 0 doesn't really mean using a cluster size of 0, which makes
> listing it as an explicit default would look odd.
>
> Inherited from old patches. Not necessary in fact. Will remove that.


> > -    .create_options     = qemu_rbd_create_options,
> > +    .create_opts        = qemu_rbd_create_opts,
>
> Hmm, I've noticed that your series is inconsistent on whether it uses '=
> foo_opts' or '= &foo_opts' in these initializers (for example, 15/26
> used & even though nothing else did, 16/26 uses & but all other
> initializers were also using &, and in this 17/26 nothing is using &).
> Not that it matters to the compiler, but there's a lot to be said for
> consistency.
>
>
Sorry, will correct that.


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

[-- Attachment #2: Type: text/html, Size: 3447 bytes --]