[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] add CephFS support in VirtFS
|
From: |
Greg Kurz |
|
Subject: |
Re: [Qemu-devel] [PATCH] add CephFS support in VirtFS |
|
Date: |
Wed, 17 Feb 2016 10:04:50 +0100 |
On Wed, 17 Feb 2016 16:49:33 +0800
Jevon Qiao <address@hidden> wrote:
> On 17/2/16 16:01, Greg Kurz wrote:
> > On Wed, 17 Feb 2016 15:32:06 +0800
> > Jevon Qiao <address@hidden> 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".
BTW, I had also answered your questions about g_new0() and a
tool to check coding style... not sure you saw that.
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
>