2014-02-18 3:01 GMT+08:00 Eric Blake <ebl...@redhat.com>:
> On 02/11/2014 11:33 PM, Chunyan Liu wrote:
> > sheepdog.c: replace QEMUOptionParameter with QemuOpts
> >
> > Signed-off-by: Dong Xu Wang <wdon...@linux.vnet.ibm.com>
> > Signed-off-by: Chunyan Liu <cy...@suse.com>
> > ---
> > block/sheepdog.c | 101
> +++++++++++++++++++++++++-----------------------------
> > 1 files changed, 47 insertions(+), 54 deletions(-)
> >
>
> > +
> > + buf = NULL;
> > + buf = qemu_opt_get_del(opts, BLOCK_OPT_REDUNDANCY);
>
> Dead NULL assignment.
>
> > + if (buf) {
> > + ret = parse_redundancy(s, buf);
> > + if (ret < 0) {
> > + goto out;
> > }
>
> Do you need to store into ret here, or is it sufficient to use 'if
> (parse_redundancy(s, buf) < 0) {'
>
> It followed the logic in existing code. If function ret < 0, return
with that return value. Same logic in earlier codes of the function.
if (strstr(filename, "://")) {
ret = sd_parse_uri(s, filename, s->name, &snapid, tag);
} else {
ret = parse_vdiname(s, filename, s->name, &snapid, tag);
}
if (ret < 0) {
goto out;
}
> > - {
> > - .name = BLOCK_OPT_REDUNDANCY,
> > - .type = OPT_STRING,
> > - .help = "Redundancy of the image"
> > - },
> > - { NULL }
> > +static QemuOptsList sd_create_opts = {
> > + .name = "sheepdog-create-opts",
> > + .head = QTAILQ_HEAD_INITIALIZER(sd_create_opts.head),
> > + .desc = {
> > + {
>
> > + .name = BLOCK_OPT_PREALLOC,
> > + .type = QEMU_OPT_STRING,
> > + .help = "Preallocation mode (allowed values: off, full)"
> > + },
> > + { /* end of list */ }
>
> Missing redundancy in the new list.
>
> > @@ -2538,7 +2533,7 @@ static BlockDriver bdrv_sheepdog = {
> > .bdrv_save_vmstate = sd_save_vmstate,
> > .bdrv_load_vmstate = sd_load_vmstate,
> >
> > - .create_options = sd_create_options,
> > + .create_opts = &sd_create_opts,
>
> Another example of the inconsistent use of &.
>
> > };
> >
> > static BlockDriver bdrv_sheepdog_tcp = {
> > @@ -2548,7 +2543,7 @@ static BlockDriver bdrv_sheepdog_tcp = {
> > .bdrv_needs_filename = true,
> > .bdrv_file_open = sd_open,
> > .bdrv_close = sd_close,
> > - .bdrv_create = sd_create,
> > + .bdrv_create2 = sd_create,
> > .bdrv_has_zero_init = bdrv_has_zero_init_1,
> > .bdrv_getlength = sd_getlength,
> > .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
> > @@ -2568,7 +2563,6 @@ static BlockDriver bdrv_sheepdog_tcp = {
> > .bdrv_save_vmstate = sd_save_vmstate,
> > .bdrv_load_vmstate = sd_load_vmstate,
> >
> > - .create_options = sd_create_options,
> > };
>
> Why no .create_opts replacement?
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>
