Eric Blake <ebl...@redhat.com> writes: > On 03/30/2017 12:43 PM, Markus Armbruster wrote: >> -blockdev and blockdev_add convert their arguments via QObject to >> BlockdevOptions for qmp_blockdev_add(), which converts them back to >> QObject, then to a flattened QDict. The QDict's members are typed >> according to the QAPI schema. >> >> -drive converts its argument via QemuOpts to a (flat) QDict. This >> QDict's members are all QString. >> >> Thus, the QType of a flat QDict member depends on whether it comes >> from -drive or -blockdev/blockdev_add, except when the QAPI type maps >> to QString, which is the case for 'str' and enumeration types. >> > >> +++ b/block/file-posix.c >> @@ -2193,6 +2193,12 @@ static int hdev_open(BlockDriverState *bs, QDict >> *options, int flags, >> int ret; >> >> #if defined(__APPLE__) && defined(__MACH__) >> + /* >> + * Caution: while qdict_get_try_str() is fine, getting non-string >> + * types would require more care. When @options come from -blockdev >> + * or blockdev_add, its members are typed according to the QAPI >> + * schema, but when they come from -drive, they're all QString. >> + */ >> const char *filename = qdict_get_str(options, "filename"); > > Comment mentions (via copy-and-past) qdict_get_try_str(), but this > instance uses qdict_get_str(). Doesn't bother me enough to withhold > review if you leave it, though. > > >> +++ b/block/nfs.c >> @@ -474,6 +474,13 @@ static NFSServer *nfs_config(QDict *options, Error >> **errp) >> goto out; >> } >> >> + /* >> + * Caution: this works only because all scalar members of >> + * NFSServer are QString in @crumpled_addr. The visitor expects >> + * @crumpled_addr to be typed according to the QAPI scherma. It > > s/scherma/schema/
In case nothing else comes up, I hope both can be touched up on commit. Max? >> + * is when @options come from -blockdev or blockdev_add. But when >> + * they come from -drive, they're all QString. >> + */ > > With the typo fix, > Reviewed-by: Eric Blake <ebl...@redhat.com> Thanks!
