On Sat, 14 Jul 2012 00:48:58 +0200
Laszlo Ersek <ler...@redhat.com> wrote:
> On 07/13/12 18:51, Luiz Capitulino wrote:
> > On Wed, 13 Jun 2012 10:22:36 +0200
> > Laszlo Ersek <ler...@redhat.com> wrote:
>
> >> Repeating an optarg is supported;
> >
> > I see that the current code supports this too, but why? Something
> > like this should fail:
> >
> > -netdev type=tap,vhost=on,vhost=off,id=guest1,script=qemu-ifup-switch
>
> > Also, you're using a queue to support the repeating of optargs,
> > right? I think this could be simplified if we just don't support
> > that.
>
> I hate repeated options with a passion, but SLIRP's hostfwd and guestfwd
> depend on repetition.
Oh, I see. I think it would be nicer to have a flag for this in QemuOptsList,
but that's unrelated to this series.
> > This doesn't insert the opts id into the hash, so opts_type_str()
> > will fail to find the id when the generated code visits it here:
> >
> > void visit_type_Netdev(Visitor *m, Netdev ** obj, const char *name, Error
> > **errp)
> > {
> > if (!error_is_set(errp)) {
> > Error *err = NULL;
> > visit_start_struct(m, (void **)obj, "Netdev", name, sizeof(Netdev),
> > &err);
> > if (!err) {
> > assert(!obj || *obj);
> > visit_type_str(m, obj ? &(*obj)->id : NULL, "id", &err);
> > <---------
> > [...]
> >
>
> *groan*
>
> You're right. opts_do_parse() makes an exception with "id" and doesn't
> call opt_set() for any occurrence of it. Would you accept the attached
> fix, split up and squashed into previous parts appropriately?
I've glanced at it and it looked fine, but I'll wait for v3 to do a full
review again.
