From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45962) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cmHAC-0002Za-0S for qemu-devel@nongnu.org; Fri, 10 Mar 2017 04:48:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cmHA9-0005iM-Bk for qemu-devel@nongnu.org; Fri, 10 Mar 2017 04:48:44 -0500 Received: from 17.mo3.mail-out.ovh.net ([87.98.178.58]:59819) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cmHA9-0005hw-0l for qemu-devel@nongnu.org; Fri, 10 Mar 2017 04:48:41 -0500 Received: from player693.ha.ovh.net (b6.ovh.net [213.186.33.56]) by mo3.mail-out.ovh.net (Postfix) with ESMTP id B7B21B48AD for ; Fri, 10 Mar 2017 10:48:37 +0100 (CET) Date: Fri, 10 Mar 2017 10:48:28 +0100 From: Greg Kurz Message-ID: <20170310104828.78604daf@bahia> In-Reply-To: <20170309185426.3506a17b@bahia> References: <1488852768-8935-1-git-send-email-sstabellini@kernel.org> <1488852768-8935-3-git-send-email-sstabellini@kernel.org> <20170309185426.3506a17b@bahia> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; boundary="Sig_/wI20=p6ac62gZp=LQwj4pOq"; protocol="application/pgp-signature" Subject: Re: [Qemu-devel] [PATCH 3/8] xen/9pfs: introduce Xen 9pfs backend List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: qemu-devel@nongnu.org, xen-devel@lists.xenproject.org, Stefano Stabellini , anthony.perard@citrix.com, jgross@suse.com, "Aneesh Kumar K.V" --Sig_/wI20=p6ac62gZp=LQwj4pOq Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Thu, 9 Mar 2017 18:54:26 +0100 Greg Kurz wrote: > On Mon, 6 Mar 2017 18:12:43 -0800 > Stefano Stabellini wrote: >=20 > > Introduce the Xen 9pfs backend: add struct XenDevOps to register as a > > Xen backend and add struct V9fsTransport to register as v9fs transport. > >=20 > > All functions are empty stubs for now. > >=20 > > Signed-off-by: Stefano Stabellini > > CC: anthony.perard@citrix.com > > CC: jgross@suse.com > > CC: Aneesh Kumar K.V > > CC: Greg Kurz > > --- > > hw/9pfs/xen-9p-backend.c | 96 ++++++++++++++++++++++++++++++++++++++++= ++++++++ > > 1 file changed, 96 insertions(+) > > create mode 100644 hw/9pfs/xen-9p-backend.c > >=20 > > diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c > > new file mode 100644 > > index 0000000..924fb64 > > --- /dev/null > > +++ b/hw/9pfs/xen-9p-backend.c > > @@ -0,0 +1,96 @@ > > +/* > > + * Xen 9p backend > > + * > > + * Copyright Aporeto 2017 > > + * > > + * Authors: > > + * Stefano Stabellini > > + * > > + */ > > + > > +#include "qemu/osdep.h" > > + > > +#include "hw/hw.h" > > +#include "hw/9pfs/9p.h" > > +#include "hw/xen/xen_backend.h" > > +#include "xen_9pfs.h" > > +#include "qemu/config-file.h" > > +#include "fsdev/qemu-fsdev.h" > > + > > +struct Xen9pfsDev { > > + struct XenDevice xendev; /* must be first */ > > +}; =20 >=20 > According to HACKING, this should be: >=20 > typedef struct Xen9pfsDev { > struct XenDevice xendev; /* must be first */ > } Xen9pfsDev; >=20 > and... >=20 > > + > > +static ssize_t xen_9pfs_pdu_vmarshal(V9fsPDU *pdu, > > + size_t offset, > > + const char *fmt, > > + va_list ap) > > +{ > > + return 0; > > +} > > + > > +static ssize_t xen_9pfs_pdu_vunmarshal(V9fsPDU *pdu, > > + size_t offset, > > + const char *fmt, > > + va_list ap) > > +{ > > + return 0; > > +} > > + > > +static void xen_9pfs_init_out_iov_from_pdu(V9fsPDU *pdu, > > + struct iovec **piov, > > + unsigned int *pniov) > > +{ > > +} > > + > > +static void xen_9pfs_init_in_iov_from_pdu(V9fsPDU *pdu, > > + struct iovec **piov, > > + unsigned int *pniov, > > + size_t size) > > +{ > > +} > > + > > +static void xen_9pfs_push_and_notify(V9fsPDU *pdu) > > +{ > > +} > > + > > +static const struct V9fsTransport xen_9p_transport =3D { > > + .pdu_vmarshal =3D xen_9pfs_pdu_vmarshal, > > + .pdu_vunmarshal =3D xen_9pfs_pdu_vunmarshal, > > + .init_in_iov_from_pdu =3D xen_9pfs_init_in_iov_from_pdu, > > + .init_out_iov_from_pdu =3D xen_9pfs_init_out_iov_from_pdu, > > + .push_and_notify =3D xen_9pfs_push_and_notify, > > +}; > > + > > +static int xen_9pfs_init(struct XenDevice *xendev) > > +{ > > + return 0; > > +} > > + > > +static int xen_9pfs_free(struct XenDevice *xendev) > > +{ > > + return -1; > > +} > > + > > +static int xen_9pfs_connect(struct XenDevice *xendev) > > +{ > > + return 0; > > +} > > + > > +static void xen_9pfs_alloc(struct XenDevice *xendev) > > +{ > > +} > > + > > +static void xen_9pfs_disconnect(struct XenDevice *xendev) > > +{ > > +} > > + > > +struct XenDevOps xen_9pfs_ops =3D { > > + .size =3D sizeof(struct Xen9pfsDev), =20 >=20 > ... s/struct // >=20 > > + .flags =3D DEVOPS_FLAG_NEED_GNTDEV, > > + .alloc =3D xen_9pfs_alloc, > > + .init =3D xen_9pfs_init, > > + .initialise =3D xen_9pfs_connect, > > + .disconnect =3D xen_9pfs_disconnect, > > + .free =3D xen_9pfs_free, > > +}; =20 >=20 > With the above comments addressed: >=20 > Reviewed-by: Greg Kurz Hmm... there's still a problem actually. This patch breaks build for some targets with '--enable-xen --enable-virtfs': LINK cris-softmmu/qemu-system-cris ../hw/xen/xen_backend.o: In function `xen_be_register_common': /home/greg/Work/qemu/qemu-9p/hw/xen/xen_backend.c:588: undefined reference = to `xen_9pfs_ops' This happens because only targets that support virtio and PCI pull the 9pfs/fsdev object files. This is an effort to have smaller QEMU binaries, based on the historical dependency of 9pfs on virtio. This patch breaks this assumption if CONFIG_XEN_BACKEND is defined but the target doesn't define CONFIG_VIRTIO and CONFIG_PCI. Since 9pfs can now be used with another transport, I guess that the dependency on virtio and PCI isn't right anymore. The new condition for targets to support 9pfs should rather be something like: CONFIG_VIRTFS =3D=3D y && CONFIG_SOFTMMU =3D=3D y && (CONFIG_VIRTIO =3D=3D = y || CONFIG_XEN_BACKEND =3D=3D y) This would cause all targets to pull the 9pfs/fsdev object files though. So I have a question: do all targets need to support the Xen backend, reall= y ? --Sig_/wI20=p6ac62gZp=LQwj4pOq Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAljCdmwACgkQAvw66wEB28JGYQCeMgjpa8np/TbGxQOoVnCUp1iG 6Y0An27cB9tqtrTMHCX+t89KTemTr+hE =4ZTa -----END PGP SIGNATURE----- --Sig_/wI20=p6ac62gZp=LQwj4pOq-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kurz Subject: Re: [PATCH 3/8] xen/9pfs: introduce Xen 9pfs backend Date: Fri, 10 Mar 2017 10:48:28 +0100 Message-ID: <20170310104828.78604daf@bahia> References: <1488852768-8935-1-git-send-email-sstabellini@kernel.org> <1488852768-8935-3-git-send-email-sstabellini@kernel.org> <20170309185426.3506a17b@bahia> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8748336555104579120==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cmHA8-00051I-OI for xen-devel@lists.xenproject.org; Fri, 10 Mar 2017 09:48:40 +0000 Received: from player693.ha.ovh.net (b6.ovh.net [213.186.33.56]) by mo3.mail-out.ovh.net (Postfix) with ESMTP id EAD7DB4861 for ; Fri, 10 Mar 2017 10:48:37 +0100 (CET) In-Reply-To: <20170309185426.3506a17b@bahia> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Stefano Stabellini Cc: jgross@suse.com, qemu-devel@nongnu.org, Stefano Stabellini , "Aneesh Kumar K.V" , anthony.perard@citrix.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org --===============8748336555104579120== Content-Type: multipart/signed; micalg=pgp-sha1; boundary="Sig_/wI20=p6ac62gZp=LQwj4pOq"; protocol="application/pgp-signature" --Sig_/wI20=p6ac62gZp=LQwj4pOq Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Thu, 9 Mar 2017 18:54:26 +0100 Greg Kurz wrote: > On Mon, 6 Mar 2017 18:12:43 -0800 > Stefano Stabellini wrote: >=20 > > Introduce the Xen 9pfs backend: add struct XenDevOps to register as a > > Xen backend and add struct V9fsTransport to register as v9fs transport. > >=20 > > All functions are empty stubs for now. > >=20 > > Signed-off-by: Stefano Stabellini > > CC: anthony.perard@citrix.com > > CC: jgross@suse.com > > CC: Aneesh Kumar K.V > > CC: Greg Kurz > > --- > > hw/9pfs/xen-9p-backend.c | 96 ++++++++++++++++++++++++++++++++++++++++= ++++++++ > > 1 file changed, 96 insertions(+) > > create mode 100644 hw/9pfs/xen-9p-backend.c > >=20 > > diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c > > new file mode 100644 > > index 0000000..924fb64 > > --- /dev/null > > +++ b/hw/9pfs/xen-9p-backend.c > > @@ -0,0 +1,96 @@ > > +/* > > + * Xen 9p backend > > + * > > + * Copyright Aporeto 2017 > > + * > > + * Authors: > > + * Stefano Stabellini > > + * > > + */ > > + > > +#include "qemu/osdep.h" > > + > > +#include "hw/hw.h" > > +#include "hw/9pfs/9p.h" > > +#include "hw/xen/xen_backend.h" > > +#include "xen_9pfs.h" > > +#include "qemu/config-file.h" > > +#include "fsdev/qemu-fsdev.h" > > + > > +struct Xen9pfsDev { > > + struct XenDevice xendev; /* must be first */ > > +}; =20 >=20 > According to HACKING, this should be: >=20 > typedef struct Xen9pfsDev { > struct XenDevice xendev; /* must be first */ > } Xen9pfsDev; >=20 > and... >=20 > > + > > +static ssize_t xen_9pfs_pdu_vmarshal(V9fsPDU *pdu, > > + size_t offset, > > + const char *fmt, > > + va_list ap) > > +{ > > + return 0; > > +} > > + > > +static ssize_t xen_9pfs_pdu_vunmarshal(V9fsPDU *pdu, > > + size_t offset, > > + const char *fmt, > > + va_list ap) > > +{ > > + return 0; > > +} > > + > > +static void xen_9pfs_init_out_iov_from_pdu(V9fsPDU *pdu, > > + struct iovec **piov, > > + unsigned int *pniov) > > +{ > > +} > > + > > +static void xen_9pfs_init_in_iov_from_pdu(V9fsPDU *pdu, > > + struct iovec **piov, > > + unsigned int *pniov, > > + size_t size) > > +{ > > +} > > + > > +static void xen_9pfs_push_and_notify(V9fsPDU *pdu) > > +{ > > +} > > + > > +static const struct V9fsTransport xen_9p_transport =3D { > > + .pdu_vmarshal =3D xen_9pfs_pdu_vmarshal, > > + .pdu_vunmarshal =3D xen_9pfs_pdu_vunmarshal, > > + .init_in_iov_from_pdu =3D xen_9pfs_init_in_iov_from_pdu, > > + .init_out_iov_from_pdu =3D xen_9pfs_init_out_iov_from_pdu, > > + .push_and_notify =3D xen_9pfs_push_and_notify, > > +}; > > + > > +static int xen_9pfs_init(struct XenDevice *xendev) > > +{ > > + return 0; > > +} > > + > > +static int xen_9pfs_free(struct XenDevice *xendev) > > +{ > > + return -1; > > +} > > + > > +static int xen_9pfs_connect(struct XenDevice *xendev) > > +{ > > + return 0; > > +} > > + > > +static void xen_9pfs_alloc(struct XenDevice *xendev) > > +{ > > +} > > + > > +static void xen_9pfs_disconnect(struct XenDevice *xendev) > > +{ > > +} > > + > > +struct XenDevOps xen_9pfs_ops =3D { > > + .size =3D sizeof(struct Xen9pfsDev), =20 >=20 > ... s/struct // >=20 > > + .flags =3D DEVOPS_FLAG_NEED_GNTDEV, > > + .alloc =3D xen_9pfs_alloc, > > + .init =3D xen_9pfs_init, > > + .initialise =3D xen_9pfs_connect, > > + .disconnect =3D xen_9pfs_disconnect, > > + .free =3D xen_9pfs_free, > > +}; =20 >=20 > With the above comments addressed: >=20 > Reviewed-by: Greg Kurz Hmm... there's still a problem actually. This patch breaks build for some targets with '--enable-xen --enable-virtfs': LINK cris-softmmu/qemu-system-cris ../hw/xen/xen_backend.o: In function `xen_be_register_common': /home/greg/Work/qemu/qemu-9p/hw/xen/xen_backend.c:588: undefined reference = to `xen_9pfs_ops' This happens because only targets that support virtio and PCI pull the 9pfs/fsdev object files. This is an effort to have smaller QEMU binaries, based on the historical dependency of 9pfs on virtio. This patch breaks this assumption if CONFIG_XEN_BACKEND is defined but the target doesn't define CONFIG_VIRTIO and CONFIG_PCI. Since 9pfs can now be used with another transport, I guess that the dependency on virtio and PCI isn't right anymore. The new condition for targets to support 9pfs should rather be something like: CONFIG_VIRTFS =3D=3D y && CONFIG_SOFTMMU =3D=3D y && (CONFIG_VIRTIO =3D=3D = y || CONFIG_XEN_BACKEND =3D=3D y) This would cause all targets to pull the 9pfs/fsdev object files though. So I have a question: do all targets need to support the Xen backend, reall= y ? --Sig_/wI20=p6ac62gZp=LQwj4pOq Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAljCdmwACgkQAvw66wEB28JGYQCeMgjpa8np/TbGxQOoVnCUp1iG 6Y0An27cB9tqtrTMHCX+t89KTemTr+hE =4ZTa -----END PGP SIGNATURE----- --Sig_/wI20=p6ac62gZp=LQwj4pOq-- --===============8748336555104579120== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============8748336555104579120==--