Re: [Qemu-devel] [PATCH RFC v2 8/9] rbd: Rewrite the code to extract list-valued options

[Eric Blake <eblake@redhat.com>]

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

On 03/24/2017 12:44 PM, Markus Armbruster wrote:
> We have two list-values options:
> 
> * "server" is a list of InetSocketAddress.  We use members "host" and
>   "port", and silently ignore the rest.
> 
> * "auth-supported" is a list of RbdAuthMethod.  We use its only member
>   "auth".
> 
> Since qemu_rbd_open() takes options as a flattened QDict, options has
> keys of the form server.%d.host, server.%d.port and
> auth-supported.%d.auth, where %d counts up from zero.
> 

> Rewrite to simply get the values straight from the options QDict.
> 
> Fixes -drive not to crash when server.*.* are present, but
> server.*.host is absent.
> 
> Permits cleaning up runtime_opts, which fixes -drive to reject several
> bogus parameters instead of silently ignoring them: host, port, auth;
> server.*.P where P isn't host; port auth-supported.*.P where P isn't

s/host; port/host or port;/

> auth.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/rbd.c | 150 +++++++++++++++++++++---------------------------------------
>  1 file changed, 52 insertions(+), 98 deletions(-)
> 
> diff --git a/block/rbd.c b/block/rbd.c
> index 6397626..e962641 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -13,6 +13,7 @@
>  
>  #include "qemu/osdep.h"
>  
> +#include <rbd/librbd.h>
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "block/block_int.h"
> @@ -20,8 +21,6 @@
>  #include "qemu/cutils.h"
>  #include "qapi/qmp/qstring.h"
>  
> -#include <rbd/librbd.h>
> -

Still no mention of the include cleanup in the commit message. Not the
end of the world (I don't think it's worth splitting into a separate
patch, and the lack of commit message mention isn't fatal).

> @@ -635,7 +590,6 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
>      }
>  
>      secretid = qemu_opt_get(opts, "password-secret");
> -
>      pool           = qemu_opt_get(opts, "pool");

Spurious change that should belong to an earlier commit?

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]