| Message ID | 1305661431-21289-7-git-send-email-jvrao@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, May 17, 2011 at 8:43 PM, Venkateswararao Jujjuri (JV) <jvrao@linux.vnet.ibm.com> wrote: > Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com> > --- > Makefile.objs | 2 +- > hw/9pfs/cofs.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > hw/9pfs/virtio-9p-coth.h | 1 + > hw/9pfs/virtio-9p.c | 27 ++++----------------------- > hw/9pfs/virtio-9p.h | 3 ++- > 5 files changed, 50 insertions(+), 25 deletions(-) > create mode 100644 hw/9pfs/cofs.c > > diff --git a/Makefile.objs b/Makefile.objs > index 96f6a24..36005bb 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y) > 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o > 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o > 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o > -9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o > +9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o > > hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y)) > $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS) > diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c > new file mode 100644 > index 0000000..6d94673 > --- /dev/null > +++ b/hw/9pfs/cofs.c > @@ -0,0 +1,42 @@ > + > +/* > + * Virtio 9p backend > + * > + * Copyright IBM, Corp. 2011 > + * > + * Authors: > + * Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + * > + */ > + > +#include "fsdev/qemu-fsdev.h" > +#include "qemu-thread.h" > +#include "qemu-coroutine.h" > +#include "virtio-9p-coth.h" > + > +int v9fs_co_readlink(V9fsState *s, V9fsString *path, V9fsString *buf) > +{ > + int err; > + ssize_t len; > + V9fsString tbuf; > + > + tbuf.data = qemu_malloc(PATH_MAX); Why introduce tbuf when the buf is available? You end up having to copy back fields at the end of the function and load from an uninitialized address (tbuf.size) in the error case. > @@ -299,7 +282,7 @@ static void v9fs_string_init(V9fsString *str) > str->size = 0; > } > > -static void v9fs_string_free(V9fsString *str) > +void v9fs_string_free(V9fsString *str) > { > qemu_free(str->data); > str->data = NULL; > @@ -421,7 +404,7 @@ v9fs_string_sprintf(V9fsString *str, const char *fmt, ...) > str->size = err; > } > > -static void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs) > +void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs) > { > v9fs_string_free(lhs); > v9fs_string_sprintf(lhs, "%s", rhs->data); Spurious changes? I don't see any users here. > @@ -1057,8 +1040,7 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name, > v9fs_string_null(&v9stat->extension); > > if (v9stat->mode & P9_STAT_MODE_SYMLINK) { > - ssize_t symlink_len; > - err = v9fs_do_readlink(s, name, &v9stat->extension, &symlink_len); > + err = v9fs_co_readlink(s, name, &v9stat->extension); > if (err < 0) { > return err; > } > @@ -3591,7 +3573,6 @@ static void v9fs_readlink(void *opaque) > int32_t fid; > int err = 0; > V9fsFidState *fidp; > - ssize_t symlink_len; > > pdu_unmarshal(pdu, offset, "d", &fid); > fidp = lookup_fid(pdu->s, fid); > @@ -3601,7 +3582,7 @@ static void v9fs_readlink(void *opaque) > } > > v9fs_string_init(&target); > - err = v9fs_do_readlink(pdu->s, &fidp->path, &target, &symlink_len); > + err = v9fs_co_readlink(pdu->s, &fidp->path, &target); > if (err < 0) { > goto out; > } Ah, the unused length argument from patch 5 has been removed. It would be nice to do fixes earlier rather than later in the patch series, otherwise time is spent reviewing and commenting on code which gets removed. Stefan
On 05/18/2011 02:43 AM, Stefan Hajnoczi wrote: > On Tue, May 17, 2011 at 8:43 PM, Venkateswararao Jujjuri (JV) > <jvrao@linux.vnet.ibm.com> wrote: >> Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com> >> --- >> Makefile.objs | 2 +- >> hw/9pfs/cofs.c | 42 ++++++++++++++++++++++++++++++++++++++++++ >> hw/9pfs/virtio-9p-coth.h | 1 + >> hw/9pfs/virtio-9p.c | 27 ++++----------------------- >> hw/9pfs/virtio-9p.h | 3 ++- >> 5 files changed, 50 insertions(+), 25 deletions(-) >> create mode 100644 hw/9pfs/cofs.c >> >> diff --git a/Makefile.objs b/Makefile.objs >> index 96f6a24..36005bb 100644 >> --- a/Makefile.objs >> +++ b/Makefile.objs >> @@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y) >> 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o >> 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o >> 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o >> -9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o >> +9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o >> >> hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y)) >> $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS) >> diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c >> new file mode 100644 >> index 0000000..6d94673 >> --- /dev/null >> +++ b/hw/9pfs/cofs.c >> @@ -0,0 +1,42 @@ >> + >> +/* >> + * Virtio 9p backend >> + * >> + * Copyright IBM, Corp. 2011 >> + * >> + * Authors: >> + * Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2. See >> + * the COPYING file in the top-level directory. >> + * >> + */ >> + >> +#include "fsdev/qemu-fsdev.h" >> +#include "qemu-thread.h" >> +#include "qemu-coroutine.h" >> +#include "virtio-9p-coth.h" >> + >> +int v9fs_co_readlink(V9fsState *s, V9fsString *path, V9fsString *buf) >> +{ >> + int err; >> + ssize_t len; >> + V9fsString tbuf; >> + >> + tbuf.data = qemu_malloc(PATH_MAX); > Why introduce tbuf when the buf is available? You end up having to > copy back fields at the end of the function and load from an > uninitialized address (tbuf.size) in the error case. tbuf is introduced for re-entrent purpose. We should be calling v9fs_string_init() on this though. >> @@ -299,7 +282,7 @@ static void v9fs_string_init(V9fsString *str) >> str->size = 0; >> } >> >> -static void v9fs_string_free(V9fsString *str) >> +void v9fs_string_free(V9fsString *str) >> { >> qemu_free(str->data); >> str->data = NULL; >> @@ -421,7 +404,7 @@ v9fs_string_sprintf(V9fsString *str, const char *fmt, ...) >> str->size = err; >> } >> >> -static void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs) >> +void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs) >> { >> v9fs_string_free(lhs); >> v9fs_string_sprintf(lhs, "%s", rhs->data); > Spurious changes? I don't see any users here. Yep. Good catch. >> @@ -1057,8 +1040,7 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name, >> v9fs_string_null(&v9stat->extension); >> >> if (v9stat->mode& P9_STAT_MODE_SYMLINK) { >> - ssize_t symlink_len; >> - err = v9fs_do_readlink(s, name,&v9stat->extension,&symlink_len); >> + err = v9fs_co_readlink(s, name,&v9stat->extension); >> if (err< 0) { >> return err; >> } >> @@ -3591,7 +3573,6 @@ static void v9fs_readlink(void *opaque) >> int32_t fid; >> int err = 0; >> V9fsFidState *fidp; >> - ssize_t symlink_len; >> >> pdu_unmarshal(pdu, offset, "d",&fid); >> fidp = lookup_fid(pdu->s, fid); >> @@ -3601,7 +3582,7 @@ static void v9fs_readlink(void *opaque) >> } >> >> v9fs_string_init(&target); >> - err = v9fs_do_readlink(pdu->s,&fidp->path,&target,&symlink_len); >> + err = v9fs_co_readlink(pdu->s,&fidp->path,&target); >> if (err< 0) { >> goto out; >> } > Ah, the unused length argument from patch 5 has been removed. It > would be nice to do fixes earlier rather than later in the patch > series, otherwise time is spent reviewing and commenting on code which > gets removed. > The previous patch is just to move errno. So we thought of doing here while we are introducing coroutines. But thinking now we could have done either place. Thanks, JV > Stefan >
On Wed, May 18, 2011 at 7:42 PM, Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com> wrote: > On 05/18/2011 02:43 AM, Stefan Hajnoczi wrote: >> >> On Tue, May 17, 2011 at 8:43 PM, Venkateswararao Jujjuri (JV) >> <jvrao@linux.vnet.ibm.com> wrote: >>> >>> Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com> >>> --- >>> Makefile.objs | 2 +- >>> hw/9pfs/cofs.c | 42 >>> ++++++++++++++++++++++++++++++++++++++++++ >>> hw/9pfs/virtio-9p-coth.h | 1 + >>> hw/9pfs/virtio-9p.c | 27 ++++----------------------- >>> hw/9pfs/virtio-9p.h | 3 ++- >>> 5 files changed, 50 insertions(+), 25 deletions(-) >>> create mode 100644 hw/9pfs/cofs.c >>> >>> diff --git a/Makefile.objs b/Makefile.objs >>> index 96f6a24..36005bb 100644 >>> --- a/Makefile.objs >>> +++ b/Makefile.objs >>> @@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y) >>> 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o >>> 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o >>> 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o >>> virtio-9p-posix-acl.o >>> -9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o >>> +9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o >>> >>> hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y)) >>> $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS) >>> diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c >>> new file mode 100644 >>> index 0000000..6d94673 >>> --- /dev/null >>> +++ b/hw/9pfs/cofs.c >>> @@ -0,0 +1,42 @@ >>> + >>> +/* >>> + * Virtio 9p backend >>> + * >>> + * Copyright IBM, Corp. 2011 >>> + * >>> + * Authors: >>> + * Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com> >>> + * >>> + * This work is licensed under the terms of the GNU GPL, version 2. See >>> + * the COPYING file in the top-level directory. >>> + * >>> + */ >>> + >>> +#include "fsdev/qemu-fsdev.h" >>> +#include "qemu-thread.h" >>> +#include "qemu-coroutine.h" >>> +#include "virtio-9p-coth.h" >>> + >>> +int v9fs_co_readlink(V9fsState *s, V9fsString *path, V9fsString *buf) >>> +{ >>> + int err; >>> + ssize_t len; >>> + V9fsString tbuf; >>> + >>> + tbuf.data = qemu_malloc(PATH_MAX); >> >> Why introduce tbuf when the buf is available? You end up having to >> copy back fields at the end of the function and load from an >> uninitialized address (tbuf.size) in the error case. > > tbuf is introduced for re-entrent purpose. > We should be calling v9fs_string_init() on this though. I see no issue here and no safety added by using a local variable. Can you explain what you're trying to do? Stefan
On 05/18/2011 10:37 PM, Stefan Hajnoczi wrote: > On Wed, May 18, 2011 at 7:42 PM, Venkateswararao Jujjuri > <jvrao@linux.vnet.ibm.com> wrote: >> On 05/18/2011 02:43 AM, Stefan Hajnoczi wrote: >>> On Tue, May 17, 2011 at 8:43 PM, Venkateswararao Jujjuri (JV) >>> <jvrao@linux.vnet.ibm.com> wrote: >>>> Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com> >>>> --- >>>> Makefile.objs | 2 +- >>>> hw/9pfs/cofs.c | 42 >>>> ++++++++++++++++++++++++++++++++++++++++++ >>>> hw/9pfs/virtio-9p-coth.h | 1 + >>>> hw/9pfs/virtio-9p.c | 27 ++++----------------------- >>>> hw/9pfs/virtio-9p.h | 3 ++- >>>> 5 files changed, 50 insertions(+), 25 deletions(-) >>>> create mode 100644 hw/9pfs/cofs.c >>>> >>>> diff --git a/Makefile.objs b/Makefile.objs >>>> index 96f6a24..36005bb 100644 >>>> --- a/Makefile.objs >>>> +++ b/Makefile.objs >>>> @@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y) >>>> 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o >>>> 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o >>>> 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o >>>> virtio-9p-posix-acl.o >>>> -9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o >>>> +9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o >>>> >>>> hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y)) >>>> $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS) >>>> diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c >>>> new file mode 100644 >>>> index 0000000..6d94673 >>>> --- /dev/null >>>> +++ b/hw/9pfs/cofs.c >>>> @@ -0,0 +1,42 @@ >>>> + >>>> +/* >>>> + * Virtio 9p backend >>>> + * >>>> + * Copyright IBM, Corp. 2011 >>>> + * >>>> + * Authors: >>>> + * Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com> >>>> + * >>>> + * This work is licensed under the terms of the GNU GPL, version 2. See >>>> + * the COPYING file in the top-level directory. >>>> + * >>>> + */ >>>> + >>>> +#include "fsdev/qemu-fsdev.h" >>>> +#include "qemu-thread.h" >>>> +#include "qemu-coroutine.h" >>>> +#include "virtio-9p-coth.h" >>>> + >>>> +int v9fs_co_readlink(V9fsState *s, V9fsString *path, V9fsString *buf) >>>> +{ >>>> + int err; >>>> + ssize_t len; >>>> + V9fsString tbuf; >>>> + >>>> + tbuf.data = qemu_malloc(PATH_MAX); >>> Why introduce tbuf when the buf is available? You end up having to >>> copy back fields at the end of the function and load from an >>> uninitialized address (tbuf.size) in the error case. >> tbuf is introduced for re-entrent purpose. >> We should be calling v9fs_string_init() on this though. > I see no issue here and no safety added by using a local variable. > Can you explain what you're trying to do? No issues in the current usage. Just looking at the micro level of this routine, we have an passd-in buffer passed under lock and is getting updated outside the lock (worker thread). Can take it out, not an issue. - JV > Stefan >
On Thu, May 19, 2011 at 4:28 PM, Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com> wrote: > On 05/18/2011 10:37 PM, Stefan Hajnoczi wrote: >> >> On Wed, May 18, 2011 at 7:42 PM, Venkateswararao Jujjuri >> <jvrao@linux.vnet.ibm.com> wrote: >>> >>> On 05/18/2011 02:43 AM, Stefan Hajnoczi wrote: >>>> >>>> On Tue, May 17, 2011 at 8:43 PM, Venkateswararao Jujjuri (JV) >>>> <jvrao@linux.vnet.ibm.com> wrote: >>>>> >>>>> Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com> >>>>> --- >>>>> Makefile.objs | 2 +- >>>>> hw/9pfs/cofs.c | 42 >>>>> ++++++++++++++++++++++++++++++++++++++++++ >>>>> hw/9pfs/virtio-9p-coth.h | 1 + >>>>> hw/9pfs/virtio-9p.c | 27 ++++----------------------- >>>>> hw/9pfs/virtio-9p.h | 3 ++- >>>>> 5 files changed, 50 insertions(+), 25 deletions(-) >>>>> create mode 100644 hw/9pfs/cofs.c >>>>> >>>>> diff --git a/Makefile.objs b/Makefile.objs >>>>> index 96f6a24..36005bb 100644 >>>>> --- a/Makefile.objs >>>>> +++ b/Makefile.objs >>>>> @@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y) >>>>> 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o >>>>> 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o >>>>> 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o >>>>> virtio-9p-posix-acl.o >>>>> -9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o >>>>> +9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o >>>>> >>>>> hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y)) >>>>> $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS) >>>>> diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c >>>>> new file mode 100644 >>>>> index 0000000..6d94673 >>>>> --- /dev/null >>>>> +++ b/hw/9pfs/cofs.c >>>>> @@ -0,0 +1,42 @@ >>>>> + >>>>> +/* >>>>> + * Virtio 9p backend >>>>> + * >>>>> + * Copyright IBM, Corp. 2011 >>>>> + * >>>>> + * Authors: >>>>> + * Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com> >>>>> + * >>>>> + * This work is licensed under the terms of the GNU GPL, version 2. >>>>> See >>>>> + * the COPYING file in the top-level directory. >>>>> + * >>>>> + */ >>>>> + >>>>> +#include "fsdev/qemu-fsdev.h" >>>>> +#include "qemu-thread.h" >>>>> +#include "qemu-coroutine.h" >>>>> +#include "virtio-9p-coth.h" >>>>> + >>>>> +int v9fs_co_readlink(V9fsState *s, V9fsString *path, V9fsString *buf) >>>>> +{ >>>>> + int err; >>>>> + ssize_t len; >>>>> + V9fsString tbuf; >>>>> + >>>>> + tbuf.data = qemu_malloc(PATH_MAX); >>>> >>>> Why introduce tbuf when the buf is available? You end up having to >>>> copy back fields at the end of the function and load from an >>>> uninitialized address (tbuf.size) in the error case. >>> >>> tbuf is introduced for re-entrent purpose. >>> We should be calling v9fs_string_init() on this though. >> >> I see no issue here and no safety added by using a local variable. >> Can you explain what you're trying to do? > > No issues in the current usage. Just looking at the micro level of this > routine, > we have an passd-in buffer passed under lock and is getting updated outside > the lock (worker thread). > Can take it out, not an issue. Please take it out since it isn't needed. Stefan
diff --git a/Makefile.objs b/Makefile.objs index 96f6a24..36005bb 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y) 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o -9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o +9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y)) $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS) diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c new file mode 100644 index 0000000..6d94673 --- /dev/null +++ b/hw/9pfs/cofs.c @@ -0,0 +1,42 @@ + +/* + * Virtio 9p backend + * + * Copyright IBM, Corp. 2011 + * + * Authors: + * Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ + +#include "fsdev/qemu-fsdev.h" +#include "qemu-thread.h" +#include "qemu-coroutine.h" +#include "virtio-9p-coth.h" + +int v9fs_co_readlink(V9fsState *s, V9fsString *path, V9fsString *buf) +{ + int err; + ssize_t len; + V9fsString tbuf; + + tbuf.data = qemu_malloc(PATH_MAX); + v9fs_co_run_in_worker( + { + len = s->ops->readlink(&s->ctx, path->data, + tbuf.data, PATH_MAX - 1); + if (len > -1) { + tbuf.size = len; + tbuf.data[len] = 0; + err = 0; + } else { + err = -errno; + } + }); + buf->size = tbuf.size; + buf->data = tbuf.data; + return err; +} diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h index 9388f9b..94c5147 100644 --- a/hw/9pfs/virtio-9p-coth.h +++ b/hw/9pfs/virtio-9p-coth.h @@ -56,4 +56,5 @@ typedef struct V9fsThPool { extern void co_run_in_worker_bh(void *); extern int v9fs_init_worker_threads(void); +extern int v9fs_co_readlink(V9fsState *, V9fsString *, V9fsString *); #endif diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c index cc22a1f..25831c3 100644 --- a/hw/9pfs/virtio-9p.c +++ b/hw/9pfs/virtio-9p.c @@ -82,23 +82,6 @@ static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf) return s->ops->lstat(&s->ctx, path->data, stbuf); } -static int v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString *buf, - ssize_t *len) -{ - int err; - buf->data = qemu_malloc(1024); - - *len = s->ops->readlink(&s->ctx, path->data, buf->data, 1024 - 1); - if (*len > -1) { - buf->size = *len; - buf->data[*len] = 0; - err = 0; - } else { - err = -errno; - } - return err; -} - static int v9fs_do_close(V9fsState *s, int fd) { return s->ops->close(&s->ctx, fd); @@ -299,7 +282,7 @@ static void v9fs_string_init(V9fsString *str) str->size = 0; } -static void v9fs_string_free(V9fsString *str) +void v9fs_string_free(V9fsString *str) { qemu_free(str->data); str->data = NULL; @@ -421,7 +404,7 @@ v9fs_string_sprintf(V9fsString *str, const char *fmt, ...) str->size = err; } -static void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs) +void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs) { v9fs_string_free(lhs); v9fs_string_sprintf(lhs, "%s", rhs->data); @@ -1057,8 +1040,7 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name, v9fs_string_null(&v9stat->extension); if (v9stat->mode & P9_STAT_MODE_SYMLINK) { - ssize_t symlink_len; - err = v9fs_do_readlink(s, name, &v9stat->extension, &symlink_len); + err = v9fs_co_readlink(s, name, &v9stat->extension); if (err < 0) { return err; } @@ -3591,7 +3573,6 @@ static void v9fs_readlink(void *opaque) int32_t fid; int err = 0; V9fsFidState *fidp; - ssize_t symlink_len; pdu_unmarshal(pdu, offset, "d", &fid); fidp = lookup_fid(pdu->s, fid); @@ -3601,7 +3582,7 @@ static void v9fs_readlink(void *opaque) } v9fs_string_init(&target); - err = v9fs_do_readlink(pdu->s, &fidp->path, &target, &symlink_len); + err = v9fs_co_readlink(pdu->s, &fidp->path, &target); if (err < 0) { goto out; } diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h index e95b63d..c6f2649 100644 --- a/hw/9pfs/virtio-9p.h +++ b/hw/9pfs/virtio-9p.h @@ -505,5 +505,6 @@ static inline size_t do_pdu_unpack(void *dst, struct iovec *sg, int sg_count, } extern void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq); - +extern void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs); +extern void v9fs_string_free(V9fsString *str); #endif
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com> --- Makefile.objs | 2 +- hw/9pfs/cofs.c | 42 ++++++++++++++++++++++++++++++++++++++++++ hw/9pfs/virtio-9p-coth.h | 1 + hw/9pfs/virtio-9p.c | 27 ++++----------------------- hw/9pfs/virtio-9p.h | 3 ++- 5 files changed, 50 insertions(+), 25 deletions(-) create mode 100644 hw/9pfs/cofs.c