Re: [Qemu-devel] [PATCH v3 2/3] 9pfs: use V9fsBlob to transmit xattr

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Jan 11, 2016 at 07:56:01PM +0530, Aneesh Kumar K.V wrote:
> Wei Liu <wei.liu2@citrix.com> writes:
> 
> > On Mon, Jan 11, 2016 at 07:26:39PM +0530, Aneesh Kumar K.V wrote:
> > [...]
> >> >      xattr_len = fidp->fs.xattr.len;
> >> >      read_count = xattr_len - off;
> >> > @@ -1596,14 +1597,18 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
> >> >           */
> >> >          read_count = 0;
> >> >      }
> >> > -    err = pdu_marshal(pdu, offset, "d", read_count);
> >> > -    if (err < 0) {
> >> > -        return err;
> >> > -    }
> >> > -    offset += err;
> >> > -    err = v9fs_pack(pdu->elem.in_sg, pdu->elem.in_num, offset,
> >> > -                    ((char *)fidp->fs.xattr.value) + off,
> >> > -                    read_count);
> >> > +
> >> > +    v9fs_blob_init(&blob);
> >> > +
> >> > +    blob.data = g_malloc(read_count);
> >> > +    memcpy(blob.data, ((char *)fidp->fs.xattr.value) + off,
> >> > +           read_count);
> >> 
> >> Can we do this without the malloc and memcpy ? . I am sure you need this
> >> for Xen abstraction. But for now i am inclined to drop this from the
> >> series and add this later with Xen transport.  v9fs_xattr_read is
> >> essentially T_READ on a xattr fid and we don't use blob ("B") in other
> >> code path. We also want to avoid that extra malloc and memcpy.
> >> 
> >
> > That's fine.
> >
> > Do you want me to resend the whole series or just this one patch
> > (assuming you don't have other comments on my other patches)?
> >
> 
> No. I did this change on top of patch 3
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index a772963fb742..3ff310605cd4 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1585,6 +1585,8 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
>      size_t offset = 7;
>      int read_count;
>      int64_t xattr_len;
> +    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> +    VirtQueueElement *elem = &v->elems[pdu->idx];
>  
>      xattr_len = fidp->fs.xattr.len;
>      read_count = xattr_len - off;
> @@ -1601,7 +1603,8 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
>          return err;
>      }
>      offset += err;
> -    err = v9fs_pack(pdu->elem.in_sg, pdu->elem.in_num, offset,
> +
> +    err = v9fs_pack(elem->in_sg, elem->in_num, offset,
>                      ((char *)fidp->fs.xattr.value) + off,
>                      read_count);
>      if (err < 0) {
> 

Sorry I don't quite follow.

Is there any concrete action I need to take?

Wei.