qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v3 for-2.9 4/9] block: Document -drive problemat


From: Eric Blake
Subject: Re: [Qemu-block] [PATCH v3 for-2.9 4/9] block: Document -drive problematic code and bugs
Date: Thu, 30 Mar 2017 13:00:55 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

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/

> +     * 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 <address@hidden>

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]