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

[Aneesh Kumar K.V]

Wei Liu <address@hidden> 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) {