Re: [Qemu-devel] [v2 2/5] block/ssh: Add InetSocketAddress and accept it

[Kevin Wolf <kwolf@redhat.com>]

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

Am 16.10.2016 um 00:30 hat Max Reitz geschrieben:
> > +static InetSocketAddress *ssh_config(BDRVSSHState *s, QDict *options,
> > +                                     Error **errp)
> > +{
> > +    InetSocketAddress *inet = NULL;
> > +    QDict *addr = NULL;
> > +    QObject *crumpled_addr = NULL;
> > +    Visitor *iv = NULL;
> > +    Error *local_error = NULL;
> > +
> > +    qdict_extract_subqdict(options, &addr, "server.");
> > +    if (!qdict_size(addr)) {
> > +        error_setg(errp, "SSH server address missing");
> > +        goto out;
> > +    }
> > +
> > +    crumpled_addr = qdict_crumple(addr, true, errp);
> > +    if (!crumpled_addr) {
> > +        goto out;
> > +    }
> > +
> > +    iv = qobject_input_visitor_new_autocast(crumpled_addr, true, 1, true);
> 
> In contrast to what Kevin said in v1, I think you do not want to use
> autocast here.
> 
> Or, to be more specific, it's difficult. The thing is that the autocast
> documentation says: "Any scalar values in the @obj input data structure
> should always be represented as strings".
> 
> So if you do use the autocast version, command line works great because
> from there everything comes as a string. But blockdev-add no longer
> works because from there everything comes with the correct type (and you
> cannot give it the wrong type).
> [...]
> In contrast, if you do not use the autocast version, blockdev-add will
> work just fine, but you can no longer specify non-string values from the
> command line.

Ah, right, I missed that. :-/

> I don't think this is your problem, though. There should be a way for
> the command line options to be converted to the correct types while we
> continue to use strict type-checking for blockdev-add.
> 
> Therefore, I think you'll have to sacrifice one or the other here. All
> of the non-string options are optional, so it won't be too bad in any case.

If we have to sacrifice one, then yes, blockdev-add is the one that must
work. The new -blockdev command line option will then automatically
work, too, so at least there will be a way to create such nodes.

The usual way to get around the type conflicts is going through a
QemuOpts. So maybe qemu_opts_from_dict() with a QemuOptionsList that
accepts anythign, and then qobject_input_visitor_new_opts() could be a
workaround to keep -drive working at the same time. It's kind of ugly,
though.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]