| Message ID | 155721870600.451636.3427944860976861371.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
| Series | fsdev/virtfs: Assorted cleanups and fixes | expand |
On 5/7/19 3:45 AM, Greg Kurz wrote: > Each fsdriver only supports a subset of the options that can be passed > to -fsdev. Unsupported options are simply ignored. This could cause the > user to erroneously think QEMU has a bug. > > Enforce strict checking of supported options for all fsdrivers. This > shouldn't impact libvirt, since it doesn't know about he synth and s/he/the/ > proxy fsdrivers. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > fsdev/qemu-fsdev.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 71 insertions(+), 3 deletions(-) > > > +#define COMMON_FS_DRIVER_OPTIONS "id", "fsdriver", "readonly" > + > static FsDriverTable FsDrivers[] = { > - { .name = "local", .ops = &local_ops}, > - { .name = "synth", .ops = &synth_ops}, > - { .name = "proxy", .ops = &proxy_ops}, > + { > + .name = "local", > + .ops = &local_ops, > + .opts = (const char * []) { > + COMMON_FS_DRIVER_OPTIONS, > + "security_model", > +static int validate_opt(void *opaque, const char *name, const char *value, > + Error **errp) > +{ > + FsDriverTable *drv = opaque; > + const char **opt; > + > + for (opt = drv->opts; *opt; opt++) { > + if (!strcmp(*opt, name)) { > + return 0; > + } > + } > + > + error_setg(errp, "'%s' is invalid for fsdriver '%s'", name, drv->name); > + return -1; > +} When we ever reach command-line QAPIfication, this might go away. In the meantime, this is an improvement. Reviewed-by: Eric Blake <eblake@redhat.com>
On Wed, 8 May 2019 11:23:46 -0500 Eric Blake <eblake@redhat.com> wrote: > On 5/7/19 3:45 AM, Greg Kurz wrote: > > Each fsdriver only supports a subset of the options that can be passed > > to -fsdev. Unsupported options are simply ignored. This could cause the > > user to erroneously think QEMU has a bug. > > > > Enforce strict checking of supported options for all fsdrivers. This > > shouldn't impact libvirt, since it doesn't know about he synth and > > s/he/the/ > > > proxy fsdrivers. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > fsdev/qemu-fsdev.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 71 insertions(+), 3 deletions(-) > > > > > > > +#define COMMON_FS_DRIVER_OPTIONS "id", "fsdriver", "readonly" > > + > > static FsDriverTable FsDrivers[] = { > > - { .name = "local", .ops = &local_ops}, > > - { .name = "synth", .ops = &synth_ops}, > > - { .name = "proxy", .ops = &proxy_ops}, > > + { > > + .name = "local", > > + .ops = &local_ops, > > + .opts = (const char * []) { > > + COMMON_FS_DRIVER_OPTIONS, > > + "security_model", > > > > +static int validate_opt(void *opaque, const char *name, const char *value, > > + Error **errp) > > +{ > > + FsDriverTable *drv = opaque; > > + const char **opt; > > + > > + for (opt = drv->opts; *opt; opt++) { > > + if (!strcmp(*opt, name)) { > > + return 0; > > + } > > + } > > + > > + error_setg(errp, "'%s' is invalid for fsdriver '%s'", name, drv->name); > > + return -1; > > +} > > When we ever reach command-line QAPIfication, this might go away. In the > meantime, this is an improvement. > > Reviewed-by: Eric Blake <eblake@redhat.com> > Fixed the typo in the changelog and applied to https://github.com/gkurz/qemu/commits/9p-next Cheers, -- Greg
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c index e972bd698cf5..077a8c4e2bca 100644 --- a/fsdev/qemu-fsdev.c +++ b/fsdev/qemu-fsdev.c @@ -34,6 +34,7 @@ typedef struct FsDriverTable { const char *name; FileOperations *ops; + const char **opts; } FsDriverTable; typedef struct FsDriverListEntry { @@ -44,12 +45,75 @@ typedef struct FsDriverListEntry { static QTAILQ_HEAD(, FsDriverListEntry) fsdriver_entries = QTAILQ_HEAD_INITIALIZER(fsdriver_entries); +#define COMMON_FS_DRIVER_OPTIONS "id", "fsdriver", "readonly" + static FsDriverTable FsDrivers[] = { - { .name = "local", .ops = &local_ops}, - { .name = "synth", .ops = &synth_ops}, - { .name = "proxy", .ops = &proxy_ops}, + { + .name = "local", + .ops = &local_ops, + .opts = (const char * []) { + COMMON_FS_DRIVER_OPTIONS, + "security_model", + "path", + "writeout", + "fmode", + "dmode", + "throttling.bps-total", + "throttling.bps-read", + "throttling.bps-write", + "throttling.iops-total", + "throttling.iops-read", + "throttling.iops-write", + "throttling.bps-total-max", + "throttling.bps-read-max", + "throttling.bps-write-max", + "throttling.iops-total-max", + "throttling.iops-read-max", + "throttling.iops-write-max", + "throttling.bps-total-max-length", + "throttling.bps-read-max-length", + "throttling.bps-write-max-length", + "throttling.iops-total-max-length", + "throttling.iops-read-max-length", + "throttling.iops-write-max-length", + "throttling.iops-size", + }, + }, + { + .name = "synth", + .ops = &synth_ops, + .opts = (const char * []) { + COMMON_FS_DRIVER_OPTIONS, + }, + }, + { + .name = "proxy", + .ops = &proxy_ops, + .opts = (const char * []) { + COMMON_FS_DRIVER_OPTIONS, + "socket", + "sock_fd", + "writeout", + }, + }, }; +static int validate_opt(void *opaque, const char *name, const char *value, + Error **errp) +{ + FsDriverTable *drv = opaque; + const char **opt; + + for (opt = drv->opts; *opt; opt++) { + if (!strcmp(*opt, name)) { + return 0; + } + } + + error_setg(errp, "'%s' is invalid for fsdriver '%s'", name, drv->name); + return -1; +} + int qemu_fsdev_add(QemuOpts *opts, Error **errp) { int i; @@ -80,6 +144,10 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp) return -1; } + if (qemu_opt_foreach(opts, validate_opt, &FsDrivers[i], errp)) { + return -1; + } + fsle = g_malloc0(sizeof(*fsle)); fsle->fse.fsdev_id = g_strdup(fsdev_id); fsle->fse.ops = FsDrivers[i].ops;
Each fsdriver only supports a subset of the options that can be passed to -fsdev. Unsupported options are simply ignored. This could cause the user to erroneously think QEMU has a bug. Enforce strict checking of supported options for all fsdrivers. This shouldn't impact libvirt, since it doesn't know about he synth and proxy fsdrivers. Signed-off-by: Greg Kurz <groug@kaod.org> --- fsdev/qemu-fsdev.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 3 deletions(-)