Aneesh Kumar K.V wrote:
> On Wed, May 26, 2010 at 04:21:40PM -0700, Venkateswararao Jujjuri (JV) wrote:
>> The new option is:
>>
>> -fsdev fstype,id=myid,path=/share_path/,security_model=[mapped|passthrough]
>> -virtfs
>> fstype,path=/share_path/,security_model=[mapped|passthrough],mnt_tag=tag
>>
>> In the case of mapped security model, files are created with QEMU user
>> credentials and the client-user's credentials are saved in extended
>> attributes.
>> Whereas in the case of passthrough security model, files on the
>> filesystem are directly created with client-user's credentials.
>>
>> Signed-off-by: Venkateswararao Jujjuri <jv...@linux.vnet.ibm.com>
>> ---
>> fsdev/qemu-fsdev.c | 14 +++++++++++++-
>> fsdev/qemu-fsdev.h | 1 +
>> hw/virtio-9p.c | 22 ++++++++++++++++++----
>> qemu-config.c | 12 +++++++++---
>> qemu-options.hx | 15 +++++++++++----
>> vl.c | 18 +++++++++++++++---
>> 6 files changed, 67 insertions(+), 15 deletions(-)
>>
>> diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
>> index 813e1f7..7d7a153 100644
>> --- a/fsdev/qemu-fsdev.c
>> +++ b/fsdev/qemu-fsdev.c
>> @@ -34,7 +34,7 @@ int qemu_fsdev_add(QemuOpts *opts)
>> return -1;
>> }
>>
>> - for (i = 0; i < ARRAY_SIZE(FsTypes); i++) {
>> + for (i = 0; i < ARRAY_SIZE(FsTypes); i++) {
>> if (strcmp(FsTypes[i].name, qemu_opt_get(opts, "fstype")) == 0) {
>> break;
>> }
>
>
> Don't do whitespace fixup in the same patch.
Ok.
>
>
>> @@ -46,10 +46,22 @@ int qemu_fsdev_add(QemuOpts *opts)
>> return -1;
>> }
>>
>> + if (qemu_opt_get(opts, "path") == NULL) {
>> + fprintf(stderr, "fsdev: No path specified.\n");
>> + return -1;
>> + }
>> +
>
>
> How is this related to new option ?
Not related. Will send out another patch.
>
>
>> + if (qemu_opt_get(opts, "security_model") == NULL) {
>> + fprintf(stderr, "fsdev: No security_model specified.\n");
>> + return -1;
>> + }
>> +
>> fsle = qemu_malloc(sizeof(*fsle));
>>
>> fsle->fse.fsdev_id = qemu_strdup(qemu_opts_id(opts));
>> fsle->fse.path = qemu_strdup(qemu_opt_get(opts, "path"));
>> + fsle->fse.security_model = qemu_strdup(qemu_opt_get(opts,
>> + "security_model"));
>> fsle->fse.ops = FsTypes[i].ops;
>>
>> QTAILQ_INSERT_TAIL(&fstype_entries, fsle, next);
>> diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
>> index b50fbe0..6c27881 100644
>> --- a/fsdev/qemu-fsdev.h
>> +++ b/fsdev/qemu-fsdev.h
>> @@ -40,6 +40,7 @@ typedef struct FsTypeTable {
>> typedef struct FsTypeEntry {
>> char *fsdev_id;
>> char *path;
>> + char *security_model;
>> FileOperations *ops;
>> } FsTypeEntry;
>>
>> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
>> index 687abc0..a57562a 100644
>> --- a/hw/virtio-9p.c
>> +++ b/hw/virtio-9p.c
>> @@ -2402,7 +2402,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev,
>> V9fsConf *conf)
>> /* We don't have a fsdev identified by fsdev_id */
>> fprintf(stderr, "Virtio-9p device couldn't find fsdev "
>> "with the id %s\n", conf->fsdev_id);
>> - exit(1);
>> + return NULL;
>> }
>>
>> if (!fse->path || !conf->tag) {
>> @@ -2410,15 +2410,29 @@ VirtIODevice *virtio_9p_init(DeviceState *dev,
>> V9fsConf *conf)
>> fprintf(stderr, "fsdev with id %s needs path "
>> "and Virtio-9p device needs mount_tag arguments\n",
>> conf->fsdev_id);
>> - exit(1);
>> + return NULL;
>> + }
>> +
>> + if (!strcmp(fse->security_model, "passthrough")) {
>> + /* Files on the Fileserver set to client user credentials */
>> + } else if (!strcmp(fse->security_model, "mapped")) {
>> + /* Files on the fileserver are set to QEMU credentials.
>> + * Client user credentials are saved in extended attributes.
>> + */
>
>
> The above two if should be dropped add them when you have something to do in
> the if () { }
> section.
>
>
>> + } else {
>> + /* user haven't specified a correct security option */
>> + fprintf(stderr, "one of the following must be specified as the"
>> + "security option:\n\t security_model=passthrough \n\t "
>> + "security_model=mapped\n");
>> + return NULL;
>> }
>
> We should only have this
Well, it is tricky. Given that we should not fix/change the code in the
previous patches
in the same series, code becomes ugly without the above place holders.
Given that I can't change this part of the code in my next patch in the
series.. I need to go like this:
if ( !strcmp(fse->security_model, "passthrough") &&
!strcmp(fse->security_model, "mapped"))
{
Error;
Return NULL;
}
Then in the next patch,
Below this i need to add in the next patch.
if (!strcmp(fse->security_model, "passthrough"))
blah
if (!strcmp(fse->security_model, "mapped"))
blah
Which makes the code to check do two srrncmps for each model.
Hence If there is no major objection.. I would like to keep it this way.
Thanks,
JV
>
>
>
>>
>> if (lstat(fse->path, &stat)) {
>> fprintf(stderr, "share path %s does not exist\n", fse->path);
>> - exit(1);
>> + return NULL;
>> } else if (!S_ISDIR(stat.st_mode)) {
>> fprintf(stderr, "share path %s is not a directory \n", fse->path);
>> - exit(1);
>> + return NULL;
>> }
>>
>> s->ctx.fs_root = qemu_strdup(fse->path);
>> diff --git a/qemu-config.c b/qemu-config.c
>> index d500885..e1e3aa1 100644
>> --- a/qemu-config.c
>> +++ b/qemu-config.c
>> @@ -160,9 +160,12 @@ QemuOptsList qemu_fsdev_opts = {
>> {
>> .name = "fstype",
>> .type = QEMU_OPT_STRING,
>> - }, {
>> + },{
>> .name = "path",
>> .type = QEMU_OPT_STRING,
>> + },{
>> + .name = "security_model",
>> + .type = QEMU_OPT_STRING,
>> },
>> { /*End of list */ }
>> },
>> @@ -178,12 +181,15 @@ QemuOptsList qemu_virtfs_opts = {
>> {
>> .name = "fstype",
>> .type = QEMU_OPT_STRING,
>> - }, {
>> + },{
>> .name = "path",
>> .type = QEMU_OPT_STRING,
>> - }, {
>> + },{
>> .name = "mount_tag",
>> .type = QEMU_OPT_STRING,
>> + },{
>> + .name = "security_model",
>> + .type = QEMU_OPT_STRING,
>> },
>>
>> { /*End of list */ }
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 12f6b51..d557c92 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -482,7 +482,7 @@ ETEXI
>> DEFHEADING(File system options:)
>>
>> DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev,
>> - "-fsdev local,id=id,path=path\n",
>> + "-fsdev local,id=id,path=path,security_model=[mapped|passthrough]\n",
>> QEMU_ARCH_ALL)
>>
>> STEXI
>> @@ -498,7 +498,7 @@ The specific Fstype will determine the applicable
>> options.
>>
>> Options to each backend are described below.
>>
>> -...@item -fsdev local ,i...@var{id} ,pa...@var{path}
>> +...@item -fsdev local ,i...@var{id} ,pa...@var{path}
>> ,security_mod...@var{security_model}
>>
>> Create a file-system-"device" for local-filesystem.
>>
>> @@ -506,6 +506,9 @@ Create a file-system-"device" for local-filesystem.
>>
>> @option{path} specifies the path to be exported. @option{path} is required.
>>
>> +...@option{security_model} specifies the security model to be followed.
>> +...@option{security_model} is required.
>> +
>> @end table
>> ETEXI
>> #endif
>> @@ -514,7 +517,7 @@ ETEXI
>> DEFHEADING(Virtual File system pass-through options:)
>>
>> DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs,
>> - "-virtfs local,path=path,mount_tag=tag\n",
>> + "-virtfs
>> local,path=path,mount_tag=tag,security_model=[mapped|passthrough]\n",
>> QEMU_ARCH_ALL)
>>
>> STEXI
>> @@ -530,7 +533,7 @@ The specific Fstype will determine the applicable
>> options.
>>
>> Options to each backend are described below.
>>
>> -...@item -virtfs local ,pa...@var{path} ,mount_t...@var{mount_tag}
>> +...@item -virtfs local ,pa...@var{path} ,mount_t...@var{mount_tag}
>> ,security_mod...@var{security_model}
>>
>> Create a Virtual file-system-pass through for local-filesystem.
>>
>> @@ -538,6 +541,10 @@ Create a Virtual file-system-pass through for
>> local-filesystem.
>>
>> @option{path} specifies the path to be exported. @option{path} is required.
>>
>> +...@option{security_model} specifies the security model to be followed.
>> +...@option{security_model} is required.
>> +
>> +
>> @option{mount_tag} specifies the tag with which the exported file is
>> mounted.
>> @option{mount_tag} is required.
>>
>> diff --git a/vl.c b/vl.c
>> index 85bcc84..a341781 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3109,10 +3109,21 @@ int main(int argc, char **argv, char **envp)
>> exit(1);
>> }
>>
>> - len = strlen(",id=,path=");
>> + if (qemu_opt_get(opts, "fstype") == NULL ||
>> + qemu_opt_get(opts, "mount_tag") == NULL ||
>> + qemu_opt_get(opts, "path") == NULL ||
>> + qemu_opt_get(opts, "security_model") == NULL) {
>> + fprintf(stderr, "Usage: -virtfs
>> fstype,path=/share_path/,"
>> + "security_model=[mapped|passthrough],"
>> + "mnt_tag=tag.\n");
>> + exit(1);
>> + }
>> +
>> + len = strlen(",id=,path=,security_model=");
>> len += strlen(qemu_opt_get(opts, "fstype"));
>> len += strlen(qemu_opt_get(opts, "mount_tag"));
>> len += strlen(qemu_opt_get(opts, "path"));
>> + len += strlen(qemu_opt_get(opts, "security_model"));
>> arg_fsdev = qemu_malloc((len + 1) * sizeof(*arg_fsdev));
>>
>> if (!arg_fsdev) {
>> @@ -3121,10 +3132,11 @@ int main(int argc, char **argv, char **envp)
>> exit(1);
>> }
>>
>> - sprintf(arg_fsdev, "%s,id=%s,path=%s",
>> + sprintf(arg_fsdev, "%s,id=%s,path=%s,security_model=%s",
>> qemu_opt_get(opts, "fstype"),
>> qemu_opt_get(opts, "mount_tag"),
>> - qemu_opt_get(opts, "path"));
>> + qemu_opt_get(opts, "path"),
>> + qemu_opt_get(opts, "security_model"));
>>
>> len = strlen("virtio-9p-pci,fsdev=,mount_tag=");
>> len += 2*strlen(qemu_opt_get(opts, "mount_tag"));
>> --
>> 1.6.5.2
>>
>>
