From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44955) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHQG9-0003EP-Lh for qemu-devel@nongnu.org; Fri, 08 Jan 2016 01:10:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHQG6-0004H6-D1 for qemu-devel@nongnu.org; Fri, 08 Jan 2016 01:10:49 -0500 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:55191) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHQG5-0004Gw-Qw for qemu-devel@nongnu.org; Fri, 08 Jan 2016 01:10:46 -0500 Received: from localhost by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 8 Jan 2016 16:10:41 +1000 Received: from d23relay07.au.ibm.com (d23relay07.au.ibm.com [9.190.26.37]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id F29FC3578055 for ; Fri, 8 Jan 2016 17:10:38 +1100 (EST) Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay07.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u086AWxK32374864 for ; Fri, 8 Jan 2016 17:10:40 +1100 Received: from d23av04.au.ibm.com (localhost [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u086A5Ml031077 for ; Fri, 8 Jan 2016 17:10:06 +1100 From: "Aneesh Kumar K.V" In-Reply-To: <1452196584-17259-28-git-send-email-wei.liu2@citrix.com> References: <1452196584-17259-1-git-send-email-wei.liu2@citrix.com> <1452196584-17259-28-git-send-email-wei.liu2@citrix.com> Date: Fri, 08 Jan 2016 11:39:37 +0530 Message-ID: <87poxca9ge.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v2 27/27] 9pfs: disentangle V9fsState List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wei Liu , qemu-devel@nongnu.org Cc: "Michael S. Tsirkin" , Alexander Graf , Christian Borntraeger , Cornelia Huck , Greg Kurz , Richard Henderson Wei Liu writes: > V9fsState now only contains generic fields. Introduce V9fsVirtioState > for virtio transport. Change virtio-pci and virtio-ccw to use > V9fsVirtioState. Handle transport enumeration in generic routines. > Few comments below > Signed-off-by: Wei Liu > --- > hw/9pfs/9p.c | 41 ++++++++++++++++++----- > hw/9pfs/9p.h | 19 +++++++---- > hw/9pfs/virtio-9p-device.c | 82 ++++++++++++++++++++++++++++------------------ > hw/9pfs/virtio-9p.h | 12 ++++++- > hw/s390x/virtio-ccw.h | 2 +- > hw/virtio/virtio-pci.h | 2 +- > 6 files changed, 109 insertions(+), 49 deletions(-) > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index 6858b21..2cf8580 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -45,7 +45,13 @@ ssize_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...) > va_list ap; > > va_start(ap, fmt); > - ret = virtio_pdu_vmarshal(pdu, offset, fmt, ap); > + switch (pdu->transport) { > + case VIRTIO: > + ret = virtio_pdu_vmarshal(pdu, offset, fmt, ap); > + break; > + default: > + ret = -1; > + } > va_end(ap); > All that switch(pdu->transport) can go in the next series along with Xen support. It is not really needed now and when we complete Xen transport we will pull that. > return ret; > @@ -57,7 +63,13 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...) > va_list ap; > > va_start(ap, fmt); > - ret = virtio_pdu_vunmarshal(pdu, offset, fmt, ap); > + switch (pdu->transport) { > + case VIRTIO: > + ret = virtio_pdu_vunmarshal(pdu, offset, fmt, ap); > + break; > + default: > + ret = -1; > + } > va_end(ap); > > return ret; > @@ -65,7 +77,11 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...) > > static void pdu_push_and_notify(V9fsPDU *pdu) > { > - virtio_9p_push_and_notify(pdu); > + switch (pdu->transport) { > + case VIRTIO: > + virtio_9p_push_and_notify(pdu); > + break; > + } > } > > static int omode_to_uflags(int8_t mode) > @@ -1696,7 +1712,11 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu, > struct iovec *iov; > unsigned int niov; > > - virtio_init_iov_from_pdu(pdu, &iov, &niov, is_write); > + switch (pdu->transport) { > + case VIRTIO: > + virtio_init_iov_from_pdu(pdu, &iov, &niov, is_write); > + break; > + } > > qemu_iovec_init_external(&elem, iov, niov); > qemu_iovec_init(qiov, niov); > @@ -3272,8 +3292,10 @@ void pdu_submit(V9fsPDU *pdu) > } > > /* Returns 0 on success, 1 on failure. */ > -int v9fs_device_realize_common(V9fsState *s, Error **errp) > +int v9fs_device_realize_common(V9fsState *s, enum p9_transport transport, > + Error **errp) > { > + V9fsVirtioState *v = container_of(s, V9fsVirtioState, state); > int i, len; > struct stat stat; > FsDriverEntry *fse; > @@ -3284,8 +3306,10 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp) > QLIST_INIT(&s->free_list); > QLIST_INIT(&s->active_list); > for (i = 0; i < (MAX_REQ - 1); i++) { > - QLIST_INSERT_HEAD(&s->free_list, &s->pdus[i], next); > - s->pdus[i].s = s; > + QLIST_INSERT_HEAD(&s->free_list, &v->pdus[i], next); > + v->pdus[i].s = s; > + v->pdus[i].idx = i; > + v->pdus[i].transport = transport; > } > > v9fs_path_init(&path); > @@ -3360,7 +3384,8 @@ out: > return rc; > } > > -void v9fs_device_unrealize_common(V9fsState *s, Error **errp) > +void v9fs_device_unrealize_common(V9fsState *s, enum p9_transport transport, > + Error **errp) > { > g_free(s->ctx.fs_root); > g_free(s->tag); > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h > index 3fe4da4..bd8588d 100644 > --- a/hw/9pfs/9p.h > +++ b/hw/9pfs/9p.h > @@ -14,6 +14,10 @@ > #include "qemu/thread.h" > #include "qemu/coroutine.h" > > +enum p9_transport { > + VIRTIO = 0x1, > +}; > + > enum { > P9_TLERROR = 6, > P9_RLERROR, > @@ -131,9 +135,10 @@ struct V9fsPDU > uint8_t id; > uint8_t cancelled; > CoQueue complete; > - VirtQueueElement elem; > struct V9fsState *s; > QLIST_ENTRY(V9fsPDU) next; > + uint32_t idx; /* index inside the array */ > + enum p9_transport transport; > }; > Can you do this change as a separate patch ? ie, Make V9fsPDU independent of virtio . Also introduce V9fsVirtioState > > @@ -205,16 +210,12 @@ struct V9fsFidState > -aneesh