From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35700) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c9vFP-00026A-1O for qemu-devel@nongnu.org; Thu, 24 Nov 2016 09:43:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c9vFL-0002v0-QW for qemu-devel@nongnu.org; Thu, 24 Nov 2016 09:43:35 -0500 Received: from 17.mo3.mail-out.ovh.net ([87.98.178.58]:47765) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c9vFL-0002tE-Gg for qemu-devel@nongnu.org; Thu, 24 Nov 2016 09:43:31 -0500 Received: from player730.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo3.mail-out.ovh.net (Postfix) with ESMTP id D6EA2620D5 for ; Thu, 24 Nov 2016 15:43:29 +0100 (CET) Date: Thu, 24 Nov 2016 15:43:18 +0100 From: Greg Kurz Message-ID: <20161124154318.15406fa4@bahia> In-Reply-To: <20161124152310.29e5e38b@bahia> References: <1479764372-29470-1-git-send-email-sstabellini@kernel.org> <1479764372-29470-2-git-send-email-sstabellini@kernel.org> <20161124093152.5afff15c@bahia> <20161124152310.29e5e38b@bahia> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/4] 9pfs: introduce transport specific callbacks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: anthony.perard@citrix.com, xen-devel@lists.xensource.com, wei.liu2@citrix.com, aneesh.kumar@linux.vnet.ibm.com, qemu-devel@nongnu.org On Thu, 24 Nov 2016 15:23:10 +0100 Greg Kurz wrote: > On Thu, 24 Nov 2016 09:31:52 +0100 > Greg Kurz wrote: > > > On Mon, 21 Nov 2016 13:39:30 -0800 > > Stefano Stabellini wrote: > > > > > Don't call virtio functions from 9pfs generic code, use generic function > > > callbacks instead. > > > > > > Signed-off-by: Stefano Stabellini > > > --- > > > > Just a couple of indentation and line over 80 characters nits. I'll fix them > > before pushing to 9p-next. > > > > Reviewed-by: Greg Kurz > > > > Hmm... second thought... > [...] > > > + > > > +static inline int v9fs_register_transport(V9fsState *s, struct V9fsTransport *t) > > > +{ > > > + if (s->transport) { > > > + return -EINVAL; > > > + } Calling v9fs_register_transport() several times for the same V9fsState looks more like a bug than an error... also, is it possible to have several 9pfs shares with different transports ? > > > + s->transport = t; > > > + return 0; > > > +} > > > + > > > #endif > > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c > > > index 1782e4a..e1a37a4 100644 > > > --- a/hw/9pfs/virtio-9p-device.c > > > +++ b/hw/9pfs/virtio-9p-device.c > > > @@ -20,7 +20,9 @@ > > > #include "hw/virtio/virtio-access.h" > > > #include "qemu/iov.h" > > > > > > -void virtio_9p_push_and_notify(V9fsPDU *pdu) > > > +static struct V9fsTransport virtio_9p_transport; > > ... shouldn't this be const ? > > > > + > > > +static void virtio_9p_push_and_notify(V9fsPDU *pdu) > > > { > > > V9fsState *s = pdu->s; > > > V9fsVirtioState *v = container_of(s, V9fsVirtioState, state); > > > @@ -126,6 +128,7 @@ static void virtio_9p_device_realize(DeviceState *dev, Error **errp) > > > v->config_size = sizeof(struct virtio_9p_config) + strlen(s->fsconf.tag); > > > virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, v->config_size); > > > v->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output); > > > + v9fs_register_transport(s, &virtio_9p_transport); > > > > > > out: > > > return; > > > @@ -148,7 +151,7 @@ static void virtio_9p_reset(VirtIODevice *vdev) > > > v9fs_reset(&v->state); > > > } > > > > > > -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset, > > > +static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset, > > > const char *fmt, va_list ap) > > > > indent > > > > > { > > > V9fsState *s = pdu->s; > > > @@ -158,7 +161,7 @@ ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset, > > > return v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap); > > > } > > > > > > -ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset, > > > +static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset, > > > const char *fmt, va_list ap) > > > > indent > > > > > { > > > V9fsState *s = pdu->s; > > > @@ -168,7 +171,7 @@ ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset, > > > return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap); > > > } > > > > > > -void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov, > > > +static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov, > > > unsigned int *pniov, bool is_write) > > > > indent > > > > > { > > > V9fsState *s = pdu->s; > > > @@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov, > > > } > > > } > > > > > > +static struct V9fsTransport virtio_9p_transport = { > > > + .pdu_vmarshal = virtio_pdu_vmarshal, > > > + .pdu_vunmarshal = virtio_pdu_vunmarshal, > > > + .init_iov_from_pdu = virtio_init_iov_from_pdu, > > > + .push_and_notify = virtio_9p_push_and_notify, > > > +}; > > > + > > > /* virtio-9p device */ > > > > > > static const VMStateDescription vmstate_virtio_9p = { > > > diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h > > > index 52c4b9d..e763da2c 100644 > > > --- a/hw/9pfs/virtio-9p.h > > > +++ b/hw/9pfs/virtio-9p.h > > > @@ -14,15 +14,6 @@ typedef struct V9fsVirtioState > > > V9fsState state; > > > } V9fsVirtioState; > > > > > > -void virtio_9p_push_and_notify(V9fsPDU *pdu); > > > - > > > -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset, > > > - const char *fmt, va_list ap); > > > -ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset, > > > - const char *fmt, va_list ap); > > > -void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov, > > > - unsigned int *pniov, bool is_write); > > > - > > > #define TYPE_VIRTIO_9P "virtio-9p-device" > > > #define VIRTIO_9P(obj) \ > > > OBJECT_CHECK(V9fsVirtioState, (obj), TYPE_VIRTIO_9P) > > > > > >