Re: [Qemu-devel] [PATCH] add CephFS support in VirtFS

[Jevon Qiao <scaleqiao@gmail.com>]

On 17/2/16 17:04, Greg Kurz wrote:
> On Wed, 17 Feb 2016 16:49:33 +0800
> Jevon Qiao <scaleqiao@gmail.com> wrote:
>
>> On 17/2/16 16:01, Greg Kurz wrote:
>>> On Wed, 17 Feb 2016 15:32:06 +0800
>>> Jevon Qiao <scaleqiao@gmail.com> wrote:
>>>   
>>>> Hi Daniel,
>>>>
>>>> Thank you for reviewing my code, please see my reply in-line.
>>>> On 15/2/16 17:17, Daniel P. Berrange wrote:
>>>>> On Sun, Feb 14, 2016 at 01:06:40PM +0800, Jevon Qiao wrote:
>>>>>> diff --git a/configure b/configure
>>>>>> index 83b40fc..cecece7 100755
>>>>>> --- a/configure
>>>>>> +++ b/configure
>>>>>> @@ -1372,6 +1377,7 @@ disabled with --disable-FEATURE, default is enabled if
>>>>>> available:
>>>>>>       vhost-net       vhost-net acceleration support
>>>>>>       spice           spice
>>>>>>       rbd             rados block device (rbd)
>>>>>> +  cephfs      Ceph File System
>>>>> Inconsistent vertical alignment with surrounding text
>>>> This is just a display issue, I'll send the patch with 'git send-email'
>>>> later after I address all the technical comments.
>>> Expect reviewers to always comment on broken formatting. If you want the
>>> review to be focused on technical aspects, the best choice is to fix the
>>> way you send patches first.
>> OK, I'll re-send the patches first.
>>
> I don't ask you to resend this patch without the fixes. I just
> wanted to put emphasis on the fact that correct formatting is
> the first thing to have in mind before sending patches, not
> just a "display issue".
Yes, I understand. I'll resend the patch with the fix which at least 
addresses Daniel's comments.
> BTW, I had also answered your questions about g_new0() and a
> tool to check coding style... not sure you saw that.
Sorry, I missed that part due to the long email:-( . I read it now, 
thank you for the answers.

Thanks,
Jevon
> Cheers.
>
> --
> Greg
>
>> Thanks,
>> Jevon
>>>>>>       libiscsi        iscsi support
>>>>>>       libnfs          nfs support
>>>>>>       smartcard       smartcard support (libcacard)
>>>>>> +/*
>>>>>> + * Helper function for cephfs_preadv and cephfs_pwritev
>>>>>> + */
>>>>>> +inline static ssize_t preadv_pwritev(struct ceph_mount_info *cmount, int
>>>>>> fd,
>>>>> Your email client is mangling long lines, here and in many other
>>>>> places in the file. Please either fix your email client to not
>>>>> insert arbitrary line breaks, or use git send-email to submit
>>>>> the patch.
>>>> Ditto.
>>>>>> +                              const struct iovec *iov, int iov_cnt,
>>>>>> +                              off_t offset, bool do_write)
>>>>>> +{
>>>>>> +    ssize_t ret = 0;
>>>>>> +    int i = 0;
>>>>> Use size_t for iterators
>>>> I'll revise the code.
>>>>>> +    size_t len = 0;
>>>>>> +    void *buf, *buftmp;
>>>>>> +    size_t bufoffset = 0;
>>>>>> +
>>>>>> +    for (; i < iov_cnt; i++) {
>>>>>> +        len += iov[i].iov_len;
>>>>>> +    }
>>>>> iov_size() does this calculation
>>>> Thanks for the suggestion.
>>>>>> +
>>>>>> +    buf = malloc(len);
>>>>> Use g_new0(uint8_t, len)
>>>> OK.
>>>>>> +    if (buf == NULL) {
>>>>>> +        errno = ENOMEM;
>>>>>> +        return -1;
>>>>>> +    }
>>>>> and don't check ENOMEM;
>>>> Any reason for this?
>>> https://developer.gnome.org/glib/unstable/glib-Memory-Allocation.html
>>>   
>>>>>> +
>>>>>> +    i = 0;
>>>>>> +    buftmp = buf;
>>>>>> +    if (do_write) {
>>>>>> +        for (i = 0; i < iov_cnt; i++) {
>>>>>> +            memcpy((buftmp + bufoffset), iov[i].iov_base, iov[i].iov_len);
>>>>>> +            bufoffset += iov[i].iov_len;
>>>>>> +        }
>>>>>> +        ret = ceph_write(cmount, fd, buf, len, offset);
>>>>>> +        if (ret <= 0) {
>>>>>> +           errno = -ret;
>>>>>> +           ret = -1;
>>>>>> +        }
>>>>>> +    } else {
>>>>>> +        ret = ceph_read(cmount, fd, buf, len, offset);
>>>>>> +        if (ret <= 0) {
>>>>>> +            errno = -ret;
>>>>>> +            ret = -1;
>>>>>> +        } else {
>>>>>> +            for (i = 0; i < iov_cnt; i++) {
>>>>>> +                memcpy(iov[i].iov_base, (buftmp + bufoffset),
>>>>>> iov[i].iov_len);
>>>>> Mangled long line again.
>>>> That's the email client issue.
>>>>>> +                bufoffset += iov[i].iov_len;
>>>>>> +            }
>>>>>> +        }
>>>>>> +    }
>>>>>> +
>>>>>> +    free(buf);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_update_file_cred(struct ceph_mount_info *cmount,
>>>>>> +                   const char *name, FsCred *credp)
>>>>> Align the parameters on following line to the '('
>>>> I will revise the code.
>>>>>> +{
>>>>>> +    int fd, ret;
>>>>>> +    fd = ceph_open(cmount, name, O_NONBLOCK | O_NOFOLLOW, credp->fc_mode);
>>>>>> +    if (fd < 0) {
>>>>>> +        return fd;
>>>>>> +    }
>>>>>> +    ret = ceph_fchown(cmount, fd, credp->fc_uid, credp->fc_gid);
>>>>>> +    if (ret < 0) {
>>>>>> +        goto err_out;
>>>>>> +    }
>>>>>> +    ret = ceph_fchmod(cmount, fd, credp->fc_mode & 07777);
>>>>>> +err_out:
>>>>>> +    close(fd);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_lstat(FsContext *fs_ctx, V9fsPath *fs_path,
>>>>>> +                        struct stat *stbuf)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_lstat");
>>>>> All of these D_CEPHFS() lines you have are really inserting trace
>>>>> points, so you should really use the QEMU trace facility instead
>>>>> of a fprintf() based macro. ie add to trace-events and then
>>>>> call the generated trace fnuction for your event. Then get rid
>>>>> of your D_CEPHFS macro.
>>>> I will revise the code.
>>>>>> +    int ret;
>>>>>> +    char *path = fs_path->data;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)fs_ctx->private;
>>>>> fs_ctx->private is 'void *' so you don't need the (struct cephfs_data *)
>>>>> cast there - 'void *' casts to anything automatically. The same issue
>>>>> in all the other functions below too.
>>>> OK, I see.
>>>>>> +    ret = ceph_lstat(cfsdata->cmount, path, stbuf);
>>>>>> +    if (ret){
>>>>>> +        errno = -ret;
>>>>>> +        ret = -1;
>>>>>> +    }
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static ssize_t cephfs_readlink(FsContext *fs_ctx, V9fsPath *fs_path,
>>>>>> +                               char *buf, size_t bufsz)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_readlink");
>>>>>> +    int ret;
>>>>>> +    char *path = fs_path->data;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)fs_ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_readlink(cfsdata->cmount, path, buf, bufsz);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_close(FsContext *ctx, V9fsFidOpenState *fs)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_close");
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data*)ctx->private;
>>>>>> +
>>>>>> +    return ceph_close(cfsdata->cmount, fs->fd);
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_closedir(FsContext *ctx, V9fsFidOpenState *fs)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_closedir");
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data*)ctx->private;
>>>>>> +
>>>>>> +    return ceph_closedir(cfsdata->cmount, (struct ceph_dir_result
>>>>>> *)fs->dir);
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_open(FsContext *ctx, V9fsPath *fs_path,
>>>>>> +                       int flags, V9fsFidOpenState *fs)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_open");
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    fs->fd = ceph_open(cfsdata->cmount, fs_path->data, flags, 0777);
>>>>>> +    return fs->fd;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_opendir(FsContext *ctx,
>>>>>> +                          V9fsPath *fs_path, V9fsFidOpenState *fs)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_opendir");
>>>>>> +    int ret;
>>>>>> +    //char buffer[PATH_MAX];
>>>>> Remove this if its really not needed
>>>> Sure.
>>>>>> +    struct ceph_dir_result *result;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data*)ctx->private;
>>>>>> +    char *path = fs_path->data;
>>>>>> +
>>>>>> +    ret = ceph_opendir(cfsdata->cmount, path, &result);
>>>>>> +    if (ret) {
>>>>>> +        fprintf(stderr, "ceph_opendir=%d\n", ret);
>>>>> Don't put in bare printfs in the code.
>>>> OK, I'll revise the code.
>>>>>> +        return ret;
>>>>>> +    }
>>>>>> +    fs->dir = (DIR *)result;
>>>>>> +    if (!fs->dir) {
>>>>>> +        fprintf(stderr, "ceph_opendir return NULL for ceph_dir_result\n");
>>>>>> +        return -1;
>>>>>> +    }
>>>>>> +    return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static void cephfs_rewinddir(FsContext *ctx, V9fsFidOpenState *fs)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_rewinddir");
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    return ceph_rewinddir(cfsdata->cmount, (struct ceph_dir_result
>>>>>> *)fs->dir);
>>>>>> +}
>>>>>> +
>>>>>> +static off_t cephfs_telldir(FsContext *ctx, V9fsFidOpenState *fs)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_telldir");
>>>>>> +    int ret;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_telldir(cfsdata->cmount, (struct ceph_dir_result *)fs->dir);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_readdir_r(FsContext *ctx, V9fsFidOpenState *fs,
>>>>>> +                            struct dirent *entry,
>>>>>> +                            struct dirent **result)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_readdir_r");
>>>>>> +    int ret;
>>>>>> +    struct dirent *tmpent;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    tmpent = entry;
>>>>>> +    ret = ceph_readdir_r(cfsdata->cmount, (struct ceph_dir_result
>>>>>> *)fs->dir,
>>>>>> +                entry);
>>>>>> +    if (ret > 0 && entry != NULL)
>>>>>> +    {
>>>>>> +        *result = entry;
>>>>>> +    } else if (!ret)
>>>>>> +    {
>>>>>> +        *result = NULL;
>>>>>> +        entry = tmpent;
>>>>>> +    }
>>>>>> +
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static void cephfs_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t off)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_seekdir");
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    return ceph_seekdir(cfsdata->cmount, (struct ceph_dir_result*)fs->dir,
>>>>>> off);
>>>>>> +}
>>>>>> +
>>>>>> +static ssize_t cephfs_preadv(FsContext *ctx, V9fsFidOpenState *fs,
>>>>>> +                             const struct iovec *iov,
>>>>>> +                             int iovcnt, off_t offset)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_preadv");
>>>>>> +    ssize_t ret = 0;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +#if defined(LIBCEPHFS_VERSION) && LIBCEPHFS_VERSION_CODE >=
>>>>>> LIBCEPHFS_VERSION(9,
>>>>>> +    0, 3)
>>>>>> +    ret = ceph_preadv(cfsdata->cmount, fs->fd, iov, iovcnt, offset);
>>>>>> +#else
>>>>>> +    if (iovcnt > 1) {
>>>>>> +    ret = preadv_pwritev(cfsdata->cmount, fs->fd, iov, iovcnt, offset, 0);
>>>>>> +    } else if (iovcnt > 0) {
>>>>>> +    ret = ceph_read(cfsdata->cmount, fs->fd, iov[0].iov_base,
>>>>>> +            iov[0].iov_len, offset);
>>>>>> +    }
>>>>> Indent lines inside the if() statement
>>>> That's a display issue, I will fix the format by sending patches with
>>>> 'git send-email'
>>>>>> +#endif
>>>>>> +
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static ssize_t cephfs_pwritev(FsContext *ctx, V9fsFidOpenState *fs,
>>>>>> +                              const struct iovec *iov,
>>>>>> +                              int iovcnt, off_t offset)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_pwritev");
>>>>>> +    ssize_t ret = 0;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +#if defined(LIBCEPHFS_VERSION) && LIBCEPHFS_VERSION_CODE >=
>>>>>> LIBCEPHFS_VERSION(9,
>>>>>> +    0, 3)
>>>>>> +    ret = ceph_pwritev(cfsdata->cmount, fs->fd, iov, iovcnt, offset);
>>>>>> +#else
>>>>>> +    if (iovcnt > 1) {
>>>>>> +    ret = preadv_pwritev(cfsdata->cmount, fs->fd, iov, iovcnt, offset, 1);
>>>>>> +    } else if (iovcnt > 0) {
>>>>>> +    ret = ceph_write(cfsdata->cmount, fs->fd, iov[0].iov_base,
>>>>>> +            iov[0].iov_len, offset);
>>>>>> +    }
>>>>> Indent lines inside the if() statement
>>>> Ditto.
>>>>>> +#endif
>>>>>> +
>>>>>> +#ifdef CONFIG_SYNC_FILE_RANGE
>>>>>> +    if (ret > 0 && ctx->export_flags & V9FS_IMMEDIATE_WRITEOUT) {
>>>>>> +        /*
>>>>>> +         * Initiate a writeback. This is not a data integrity sync.
>>>>>> +         * We want to ensure that we don't leave dirty pages in the cache
>>>>>> +         * after write when writeout=immediate is sepcified.
>>>>>> +         */
>>>>>> +        sync_file_range(fs->fd, offset, ret,
>>>>>> +                        SYNC_FILE_RANGE_WAIT_BEFORE |
>>>>>> SYNC_FILE_RANGE_WRITE);
>>>>>> +    }
>>>>>> +#endif
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred
>>>>>> *credp)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_chmod");
>>>>>> +    int  ret = -1;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)fs_ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_chmod(cfsdata->cmount, fs_path->data, credp->fc_mode);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
>>>>>> +                       const char *name, FsCred *credp)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_mknod");
>>>>>> +    int ret;
>>>>>> +    V9fsString fullname;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)fs_ctx->private;
>>>>>> +
>>>>>> +    v9fs_string_init(&fullname);
>>>>>> +    v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
>>>>>> +    ret = ceph_mknod(cfsdata->cmount, fullname.data, credp->fc_mode,
>>>>>> +            credp->fc_rdev);
>>>>>> +
>>>>>> +    v9fs_string_free(&fullname);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
>>>>>> +                       const char *name, FsCred *credp)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_mkdir");
>>>>>> +    int ret;
>>>>>> +    V9fsString fullname;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)fs_ctx->private;
>>>>>> +
>>>>>> +    v9fs_string_init(&fullname);
>>>>>> +    v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
>>>>>> +    ret = ceph_mkdir(cfsdata->cmount, fullname.data, credp->fc_mode);
>>>>>> +
>>>>>> +    v9fs_string_free(&fullname);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_fstat(FsContext *fs_ctx, int fid_type,
>>>>>> +                        V9fsFidOpenState *fs, struct stat *stbuf)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_fstat");
>>>>>> +    int fd = -1;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)fs_ctx->private;
>>>>>> +
>>>>>> +    if (fid_type == P9_FID_DIR) {
>>>>>> +        fd = dirfd(fs->dir);
>>>>>> +    } else {
>>>>>> +        fd = fs->fd;
>>>>>> +    }
>>>>>> +    return ceph_fstat(cfsdata->cmount, fd, stbuf);
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char
>>>>>> *name,
>>>>>> +                        int flags, FsCred *credp, V9fsFidOpenState *fs)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_open2");
>>>>>> +    int fd = -1, ret = -1;
>>>>>> +    V9fsString fullname;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)fs_ctx->private;
>>>>>> +
>>>>>> +    v9fs_string_init(&fullname);
>>>>>> +    v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
>>>>>> +    fd = ceph_open(cfsdata->cmount, fullname.data, flags, credp->fc_mode);
>>>>>> +    if (fd >= 0) {
>>>>>> +        /* After creating the file, need to set the cred */
>>>>>> +        ret = cephfs_update_file_cred(cfsdata->cmount, name, credp);
>>>>>> +        if (ret < 0) {
>>>>>> +            ceph_close(cfsdata->cmount, fd);
>>>>>> +            errno = -ret;
>>>>>> +            fd = ret;
>>>>>> +        } else {
>>>>>> +            fs->fd = fd;
>>>>>> +        }
>>>>>> +    } else {
>>>>>> +       errno = -fd;
>>>>>> +    }
>>>>>> +
>>>>>> +    v9fs_string_free(&fullname);
>>>>>> +    return fd;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_symlink(FsContext *fs_ctx, const char *oldpath,
>>>>>> +                          V9fsPath *dir_path, const char *name, FsCred
>>>>>> *credp)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_symlink");
>>>>>> +    int ret = -1;
>>>>>> +    V9fsString fullname;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)fs_ctx->private;
>>>>>> +
>>>>>> +    v9fs_string_init(&fullname);
>>>>>> +    v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
>>>>>> +    ret = ceph_symlink(cfsdata->cmount, oldpath, fullname.data);
>>>>>> +
>>>>>> +    v9fs_string_free(&fullname);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_link(FsContext *ctx, V9fsPath *oldpath,
>>>>>> +                       V9fsPath *dirpath, const char *name)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_link");
>>>>>> +    int ret = -1;
>>>>>> +    V9fsString newpath;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    v9fs_string_init(&newpath);
>>>>>> +    v9fs_string_sprintf(&newpath, "%s/%s", dirpath->data, name);
>>>>>> +    ret = ceph_link(cfsdata->cmount, oldpath->data, newpath.data);
>>>>>> +
>>>>>> +    v9fs_string_free(&newpath);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_truncate");
>>>>>> +    int ret = -1;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_truncate(cfsdata->cmount, fs_path->data, size);
>>>>>> +
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_rename(FsContext *ctx, const char *oldpath,
>>>>>> +                         const char *newpath)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_rename");
>>>>>> +    int ret = -1;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_rename(cfsdata->cmount, oldpath, newpath);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred
>>>>>> *credp)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_chown");
>>>>>> +    int ret = -1;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)fs_ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_chown(cfsdata->cmount, fs_path->data, credp->fc_uid,
>>>>>> +            credp->fc_gid);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_utimensat(FsContext *ctx, V9fsPath *fs_path,
>>>>>> +                            const struct timespec *buf)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_utimensat");
>>>>>> +    int ret = -1;
>>>>>> +
>>>>>> +#ifdef CONFIG_UTIMENSAT
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_utime(cfsdata->cmount, fs_path->data, (struct utimbuf
>>>>>> *)buf);
>>>>>> +#else
>>>>>> +    ret = -1;
>>>>>> +    errno = ENOSYS;
>>>>>> +#endif
>>>>>> +
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_remove(FsContext *ctx, const char *path)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_remove");
>>>>>> +    errno = EOPNOTSUPP;
>>>>>> +    return -1;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_fsync(FsContext *ctx, int fid_type,
>>>>>> +                        V9fsFidOpenState *fs, int datasync)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_fsync");
>>>>>> +    int ret = -1, fd = -1;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    if (fid_type == P9_FID_DIR) {
>>>>>> +        fd = dirfd(fs->dir);
>>>>>> +    } else {
>>>>>> +        fd = fs->fd;
>>>>>> +    }
>>>>>> +
>>>>>> +    ret = ceph_fsync(cfsdata->cmount, fd, datasync);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_statfs(FsContext *ctx, V9fsPath *fs_path,
>>>>>> +                         struct statfs *stbuf)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_statfs");
>>>>>> +    int ret;
>>>>>> +    char *path = fs_path->data;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_statfs(cfsdata->cmount, path, (struct statvfs*)stbuf);
>>>>>> +    if (ret) {
>>>>>> +        fprintf(stderr, "ceph_statfs=%d\n", ret);
>>>>>> +    }
>>>>>> +
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +/*
>>>>>> + * Get the extended attribute of normal file, if the path refer to a
>>>>>> symbolic
>>>>>> + * link, just return the extended attributes of the syslink rather than the
>>>>>> + * attributes of the link itself.
>>>>>> + */
>>>>>> +static ssize_t cephfs_lgetxattr(FsContext *ctx, V9fsPath *fs_path,
>>>>>> +                                const char *name, void *value, size_t size)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_lgetxattr");
>>>>>> +    int ret;
>>>>>> +    char *path = fs_path->data;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_lgetxattr(cfsdata->cmount, path, name, value, size);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static ssize_t cephfs_llistxattr(FsContext *ctx, V9fsPath *fs_path,
>>>>>> +                                 void *value, size_t size)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_llistxattr");
>>>>>> +    int ret = -1;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_llistxattr(cfsdata->cmount, fs_path->data, value, size);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_lsetxattr(FsContext *ctx, V9fsPath *fs_path, const char
>>>>>> *name,
>>>>>> +                            void *value, size_t size, int flags)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_lsetxattr");
>>>>>> +    int ret = -1;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_lsetxattr(cfsdata->cmount, fs_path->data, name, value, size,
>>>>>> +    flags);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_lremovexattr(FsContext *ctx, V9fsPath *fs_path,
>>>>>> +                               const char *name)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_lremovexattr");
>>>>>> +    int ret = -1;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_lremovexattr(cfsdata->cmount, fs_path->data, name);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_name_to_path(FsContext *ctx, V9fsPath *dir_path,
>>>>>> +                              const char *name, V9fsPath *target)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_name_to_path");
>>>>>> +    if (dir_path) {
>>>>>> +        v9fs_string_sprintf((V9fsString *)target, "%s/%s",
>>>>>> +                            dir_path->data, name);
>>>>>> +    } else {
>>>>>> +        /* if the path does not start from '/' */
>>>>>> +        v9fs_string_sprintf((V9fsString *)target, "%s", name);
>>>>>> +    }
>>>>>> +
>>>>>> +    /* Bump the size for including terminating NULL */
>>>>>> +    target->size++;
>>>>>> +    return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_renameat(FsContext *ctx, V9fsPath *olddir,
>>>>>> +                           const char *old_name, V9fsPath *newdir,
>>>>>> +                           const char *new_name)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_renameat");
>>>>>> +    int ret = -1;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    ret = ceph_rename(cfsdata->cmount, old_name, new_name);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_unlinkat(FsContext *ctx, V9fsPath *dir,
>>>>>> +                           const char *name, int flags)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_unlinkat");
>>>>>> +    int ret = 0;
>>>>>> +    char *path = dir->data;
>>>>>> +    struct stat fstat;
>>>>>> +    V9fsString fullname;
>>>>>> +    struct cephfs_data *cfsdata = (struct cephfs_data *)ctx->private;
>>>>>> +
>>>>>> +    v9fs_string_init(&fullname);
>>>>>> +    v9fs_string_sprintf(&fullname, "%s/%s", dir->data, name);
>>>>>> +    path = fullname.data;
>>>>>> +    /* determine which kind of file is being destroyed */
>>>>>> +    ret = ceph_lstat(cfsdata->cmount, path, &fstat);
>>>>>> +    if (!ret) {
>>>>>> +        switch (fstat.st_mode & S_IFMT) {
>>>>>> +        case S_IFDIR:
>>>>>> +            ret = ceph_rmdir(cfsdata->cmount, path);
>>>>>> +            break;
>>>>>> +
>>>>>> +        case S_IFBLK:
>>>>>> +        case S_IFCHR:
>>>>>> +        case S_IFIFO:
>>>>>> +        case S_IFLNK:
>>>>>> +        case S_IFREG:
>>>>>> +        case S_IFSOCK:
>>>>>> +            ret = ceph_unlink(cfsdata->cmount, path);
>>>>>> +            break;
>>>>>> +
>>>>>> +        default:
>>>>>> +            fprintf(stderr, "ceph_lstat unknown stmode\n");
>>>>>> +            break;
>>>>>> +        }
>>>>>> +    } else {
>>>>>> +        errno = -ret;
>>>>>> +        ret = -1;
>>>>>> +    }
>>>>>> +
>>>>>> +    v9fs_string_free(&fullname);
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +/*
>>>>>> + * Do two things in the init function:
>>>>>> + * 1) Create a mount handle used by all cephfs interfaces.
>>>>>> + * 2) Invoke ceph_mount() to initialize a link between the client and
>>>>>> + *    ceph monitor
>>>>>> + */
>>>>>> +static int cephfs_init(FsContext *ctx)
>>>>>> +{
>>>>>> +    D_CEPHFS("cephfs_init");
>>>>>> +    int ret;
>>>>>> +    const char *ver = NULL;
>>>>>> +    struct cephfs_data *data = g_malloc(sizeof(struct cephfs_data));
>>>>>> +
>>>>>> +    if (data == NULL) {
>>>>>> +    errno = ENOMEM;
>>>>>> +    return -1;
>>>>>> +    }
>>>>>> +    memset(data, 0, sizeof(struct cephfs_data));
>>>>>> +    ret = ceph_create(&data->cmount, NULL);
>>>>>> +    if (ret) {
>>>>>> +        fprintf(stderr, "ceph_create=%d\n", ret);
>>>>>> +        goto err_out;
>>>>>> +    }
>>>>>> +
>>>>>> +    ret = ceph_conf_read_file(data->cmount, NULL);
>>>>>> +    if (ret) {
>>>>>> +        fprintf(stderr, "ceph_conf_read_file=%d\n", ret);
>>>>>> +        goto err_out;
>>>>>> +    }
>>>>>> +
>>>>>> +    ret = ceph_mount(data->cmount, ctx->fs_root);
>>>>>> +    if (ret) {
>>>>>> +        fprintf(stderr, "ceph_mount=%d\n", ret);
>>>>>> +        goto err_out;
>>>>>> +    } else {
>>>>>> +        ctx->private = data;
>>>>>> +    /* CephFS does not support FS_IOC_GETVERSIO */
>>>>>> +    ctx->exops.get_st_gen = NULL;
>>>>> Bad indent.
>>>>>      
>>>>>> +        goto out;
>>>>>> +    }
>>>>>> +
>>>>>> +    ver = ceph_version(&data->major, &data->minor, &data->patch);
>>>>>> +    memcpy(data->ceph_version, ver, strlen(ver) + 1);
>>>>>> +
>>>>>> +err_out:
>>>>>> +    g_free(data);
>>>>>> +out:
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +
>>>>>> +static int cephfs_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
>>>>>> +{
>>>>>> +    const char *sec_model = qemu_opt_get(opts, "security_model");
>>>>>> +    const char *path = qemu_opt_get(opts, "path");
>>>>>> +
>>>>>> +    if (!sec_model) {
>>>>>> +        fprintf(stderr, "Invalid argument security_model specified with "
>>>>>> +        "cephfs fsdriver\n");
>>>>> Bad indent.
>>>> BTW, is there any tool I can use to check the coding style in Qemu?
>>>>   
>>> ./scripts/checkpatch.pl is your friend.
>>>
>>>   
>>>> Thanks,
>>>> Jevon
>>>>>> +        return -1;
>>>>>> +    }
>>>>>> +
>>>>>> +    if (!path) {
>>>>>> +        fprintf(stderr, "fsdev: No path specified.\n");
>>>>>> +        return -1;
>>>>>> +    }
>>>>>> +
>>>>>> +    fse->path = g_strdup(path);
>>>>>> +    return 0;
>>>>>> +}
>>>>> Regards,
>>>>> Daniel