| Message ID | 871655967d4f29e1357abe088df38270e95f5f19.1488220970.git.jcody@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/27/2017 12:58 PM, Jeff Cody wrote: > This adds all the currently supported runtime opts, which > are the options as parsed from the filename. All of these > options are explicitly checked for during during runtime, > with an exception to the "keyvalue-pairs" option. > > This option contains all the key/value pairs that the QEMU rbd > driver merely unescapes, and passes along blindly to rados. Maybe worth adding a comment that keyvalue-pairs will NOT be exposed in QAPI in the later patches, making it command-line only and non-introspectible. > > Signed-off-by: Jeff Cody <jcody@redhat.com> > --- > block/rbd.c | 62 ++++++++++++++++++++++++++++++++++++++++++------------------- > 1 file changed, 43 insertions(+), 19 deletions(-) > > +static QemuOptsList runtime_opts = { > + .name = "rbd", > + .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), > + .desc = { > + { > + .name = "filename", > + .type = QEMU_OPT_STRING, > + .help = "Specification of the rbd image", > + }, > + { > + .name = "password-secret", > + .type = QEMU_OPT_STRING, > + .help = "ID of secret providing the password", > + }, > + { > + .name = "conf", Is "conf" the best name, or do we want "configuration"? > + .type = QEMU_OPT_STRING, > + }, Worth documenting all the options? I'm not seeing where "keyvalue-pairs" is used yet, but assume it is in a later patch. But assuming the QAPI version in a later patch matches, other than keyvalue-pairs, I think you're okay. Reviewed-by: Eric Blake <eblake@redhat.com>
On Mon, Feb 27, 2017 at 04:18:57PM -0600, Eric Blake wrote: > On 02/27/2017 12:58 PM, Jeff Cody wrote: > > This adds all the currently supported runtime opts, which > > are the options as parsed from the filename. All of these > > options are explicitly checked for during during runtime, > > with an exception to the "keyvalue-pairs" option. > > > > This option contains all the key/value pairs that the QEMU rbd > > driver merely unescapes, and passes along blindly to rados. > > Maybe worth adding a comment that keyvalue-pairs will NOT be exposed in > QAPI in the later patches, making it command-line only and > non-introspectible. > Yes, I will do that. > > > > Signed-off-by: Jeff Cody <jcody@redhat.com> > > --- > > block/rbd.c | 62 ++++++++++++++++++++++++++++++++++++++++++------------------- > > 1 file changed, 43 insertions(+), 19 deletions(-) > > > > > +static QemuOptsList runtime_opts = { > > + .name = "rbd", > > + .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), > > + .desc = { > > + { > > + .name = "filename", > > + .type = QEMU_OPT_STRING, > > + .help = "Specification of the rbd image", > > + }, > > + { > > + .name = "password-secret", > > + .type = QEMU_OPT_STRING, > > + .help = "ID of secret providing the password", > > + }, > > + { > > + .name = "conf", > > Is "conf" the best name, or do we want "configuration"? > I chose "conf" because it matches the rados option name (and the command line option name; it is of the form "conf=filename"). > > + .type = QEMU_OPT_STRING, > > + }, > > Worth documenting all the options? > Yes, probably so - and especially to map them up with what rados/ceph options they correspond to. > I'm not seeing where "keyvalue-pairs" is used yet, but assume it is in a > later patch. But assuming the QAPI version in a later patch matches, > other than keyvalue-pairs, I think you're okay. > Yep, the next patch (where we switch over to .bdrv_parse_filename()). > Reviewed-by: Eric Blake <eblake@redhat.com> > Thanks! -Jeff
diff --git a/block/rbd.c b/block/rbd.c index 33c21d8..ff5def4 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -357,6 +357,49 @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs) } } +static QemuOptsList runtime_opts = { + .name = "rbd", + .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), + .desc = { + { + .name = "filename", + .type = QEMU_OPT_STRING, + .help = "Specification of the rbd image", + }, + { + .name = "password-secret", + .type = QEMU_OPT_STRING, + .help = "ID of secret providing the password", + }, + { + .name = "conf", + .type = QEMU_OPT_STRING, + }, + { + .name = "pool", + .type = QEMU_OPT_STRING, + }, + { + .name = "image", + .type = QEMU_OPT_STRING, + }, + { + .name = "snapshot", + .type = QEMU_OPT_STRING, + }, + { + /* maps to 'id' in rados_create() */ + .name = "user", + .type = QEMU_OPT_STRING, + }, + { + .name = "keyvalue-pairs", + .type = QEMU_OPT_STRING, + }, + { /* end of list */ } + }, +}; + static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp) { Error *local_err = NULL; @@ -500,25 +543,6 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb) qemu_aio_unref(acb); } -/* TODO Convert to fine grained options */ -static QemuOptsList runtime_opts = { - .name = "rbd", - .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), - .desc = { - { - .name = "filename", - .type = QEMU_OPT_STRING, - .help = "Specification of the rbd image", - }, - { - .name = "password-secret", - .type = QEMU_OPT_STRING, - .help = "ID of secret providing the password", - }, - { /* end of list */ } - }, -}; - static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) {
This adds all the currently supported runtime opts, which are the options as parsed from the filename. All of these options are explicitly checked for during during runtime, with an exception to the "keyvalue-pairs" option. This option contains all the key/value pairs that the QEMU rbd driver merely unescapes, and passes along blindly to rados. Signed-off-by: Jeff Cody <jcody@redhat.com> --- block/rbd.c | 62 ++++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 19 deletions(-)