From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=51232 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OHNSQ-0000LJ-7S for qemu-devel@nongnu.org; Wed, 26 May 2010 16:44:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OHNSN-0001ot-Lh for qemu-devel@nongnu.org; Wed, 26 May 2010 16:44:05 -0400 Received: from mail-px0-f173.google.com ([209.85.212.173]:49242) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OHNSN-0001og-B4 for qemu-devel@nongnu.org; Wed, 26 May 2010 16:44:03 -0400 Received: by pxi2 with SMTP id 2so22289pxi.4 for ; Wed, 26 May 2010 13:44:02 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4BFD6236.6040707@linux.vnet.ibm.com> References: <4BFAE58E.1020406@linux.vnet.ibm.com> <4BFD6236.6040707@linux.vnet.ibm.com> From: Blue Swirl Date: Wed, 26 May 2010 20:43:42 +0000 Message-ID: Subject: Re: [Qemu-devel] [PATCH 1/2] virtio-9p: make virtio-9p available to all POSIX systems Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Venkateswararao Jujjuri (JV)" Cc: qemu-devel On Wed, May 26, 2010 at 6:02 PM, Venkateswararao Jujjuri (JV) wrote: > Blue Swirl wrote: >> On Mon, May 24, 2010 at 8:46 PM, Venkateswararao Jujjuri (JV) >> wrote: >>> Blue Swirl wrote: >>>> Field d_off in struct dirent is Linux specific. >>>> >>>> Signed-off-by: Blue Swirl >>>> --- >>>> =C2=A0Makefile.objs =C2=A0 | =C2=A0 =C2=A08 ++++---- >>>> =C2=A0Makefile.target | =C2=A0 =C2=A02 +- >>>> =C2=A0hw/virtio-9p.c =C2=A0| =C2=A0 =C2=A02 +- >>>> =C2=A0hw/virtio-pci.c | =C2=A0 =C2=A06 +++--- >>>> =C2=A0hw/virtio.h =C2=A0 =C2=A0 | =C2=A0 =C2=A04 ++-- >>>> =C2=A0qemu-config.c =C2=A0 | =C2=A0 =C2=A04 ++-- >>>> =C2=A0qemu-config.h =C2=A0 | =C2=A0 =C2=A02 +- >>>> =C2=A0qemu-options.hx | =C2=A0 =C2=A08 ++++---- >>>> =C2=A0vl.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A08 += +++---- >>>> =C2=A09 files changed, 22 insertions(+), 22 deletions(-) >>>> >>>> diff --git a/Makefile.objs b/Makefile.objs >>>> index 1585101..b1a6e01 100644 >>>> --- a/Makefile.objs >>>> +++ b/Makefile.objs >>>> @@ -35,8 +35,8 @@ net-nested-$(CONFIG_SLIRP) +=3D slirp.o >>>> =C2=A0net-nested-$(CONFIG_VDE) +=3D vde.o >>>> =C2=A0net-obj-y +=3D $(addprefix net/, $(net-nested-y)) >>>> >>>> -fsdev-nested-$(CONFIG_LINUX) =3D qemu-fsdev.o >>>> -fsdev-obj-$(CONFIG_LINUX) +=3D $(addprefix fsdev/, $(fsdev-nested-y)) >>>> +fsdev-nested-$(CONFIG_POSIX) =3D qemu-fsdev.o >>>> +fsdev-obj-$(CONFIG_POSIX) +=3D $(addprefix fsdev/, $(fsdev-nested-y)) >>>> >>>> =C2=A0################################################################= ###### >>>> =C2=A0# libqemu_common.a: Target independent part of system emulation.= The >>>> @@ -47,7 +47,7 @@ fsdev-obj-$(CONFIG_LINUX) +=3D $(addprefix fsdev/, >>>> $(fsdev-nested-y)) >>>> =C2=A0common-obj-y =3D $(block-obj-y) >>>> =C2=A0common-obj-y +=3D $(net-obj-y) >>>> =C2=A0common-obj-y +=3D $(qobject-obj-y) >>>> -common-obj-$(CONFIG_LINUX) +=3D $(fsdev-obj-$(CONFIG_LINUX)) >>>> +common-obj-$(CONFIG_POSIX) +=3D $(fsdev-obj-$(CONFIG_POSIX)) >>>> =C2=A0common-obj-y +=3D readline.o console.o async.o qemu-error.o >>>> =C2=A0common-obj-y +=3D tcg-runtime.o host-utils.o >>>> =C2=A0common-obj-y +=3D irq.o ioport.o input.o >>>> @@ -229,7 +229,7 @@ sound-obj-$(CONFIG_CS4231A) +=3D cs4231a.o >>>> =C2=A0adlib.o fmopl.o: QEMU_CFLAGS +=3D -DBUILD_Y8950=3D0 >>>> =C2=A0hw-obj-$(CONFIG_SOUND) +=3D $(sound-obj-y) >>>> >>>> -hw-obj-$(CONFIG_LINUX) +=3D virtio-9p-debug.o virtio-9p-local.o >>>> +hw-obj-$(CONFIG_POSIX) +=3D virtio-9p-debug.o virtio-9p-local.o >>>> >>>> =C2=A0################################################################= ###### >>>> =C2=A0# libdis >>>> diff --git a/Makefile.target b/Makefile.target >>>> index fda5bf3..00e140f 100644 >>>> --- a/Makefile.target >>>> +++ b/Makefile.target >>>> @@ -168,7 +168,7 @@ obj-y +=3D virtio-blk.o virtio-balloon.o >>>> virtio-net.o virtio-serial-bus.o >>>> =C2=A0obj-$(CONFIG_VIRTIO_PCI) +=3D virtio-pci.o >>>> =C2=A0obj-y +=3D vhost_net.o >>>> =C2=A0obj-$(CONFIG_VHOST_NET) +=3D vhost.o >>>> -obj-$(CONFIG_LINUX) +=3D virtio-9p.o >>>> +obj-$(CONFIG_POSIX) +=3D virtio-9p.o >>>> =C2=A0obj-y +=3D rwhandler.o >>>> =C2=A0obj-$(CONFIG_KVM) +=3D kvm.o kvm-all.o >>>> =C2=A0obj-$(CONFIG_NO_KVM) +=3D kvm-stub.o >>>> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c >>>> index e5d0112..68b0696 100644 >>>> --- a/hw/virtio-9p.c >>>> +++ b/hw/virtio-9p.c >>>> @@ -1447,8 +1447,8 @@ static void v9fs_read_post_dir_lstat(V9fsState >>>> *s, V9fsReadState *vs, >>>> =C2=A0 =C2=A0 =C2=A0vs->count +=3D vs->len; >>>> =C2=A0 =C2=A0 =C2=A0v9fs_stat_free(&vs->v9stat); >>>> =C2=A0 =C2=A0 =C2=A0v9fs_string_free(&vs->name); >>>> - =C2=A0 =C2=A0vs->dir_pos =3D vs->dent->d_off; >>>> =C2=A0 =C2=A0 =C2=A0vs->dent =3D v9fs_do_readdir(s, vs->fidp->dir); >>>> + =C2=A0 =C2=A0vs->dir_pos =3D v9fs_do_telldir(s, vs->fidp->dir); >>> >>> We need to save the the current dir position before making next readdir >>> We need to seek back if we can't fit it into PDU. >>> Hence moving the dir_pos after readdir is not a good idea. >> >> Hmm, the manual page for readdir says: >> =C2=A0 =C2=A0 =C2=A0 =C2=A0On Linux, the dirent structure is defined as = follows: >> >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0struct dirent { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ino_t =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0d_ino; =C2=A0 =C2=A0 =C2=A0 /* inode number */ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0off_t =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0d_off; =C2=A0 =C2=A0 =C2=A0 /* offset to the next d= irent */ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned short d_= reclen; =C2=A0 =C2=A0/* length of this record */ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned char =C2= =A0d_type; =C2=A0 =C2=A0 =C2=A0/* type of file */ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0char =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 d_name[256]; /* filename */ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}; >> >> My change was based on the comment for d_off. >> >> But when I run this program: >> >> $ cat dirent.c >> #include >> #include >> #include >> >> int main(int argc, const char **argv) >> { >> =C2=A0 =C2=A0 DIR *d; >> =C2=A0 =C2=A0 struct dirent *entry; >> =C2=A0 =C2=A0 off_t pos; >> >> =C2=A0 =C2=A0 d =3D opendir(argv[1]); >> =C2=A0 =C2=A0 entry =3D readdir(d); >> =C2=A0 =C2=A0 do { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 pos =3D telldir(d); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 printf("name %s d_off %ld ino %ld pos %ld\n"= , entry->d_name, >> entry->d_off, entry->d_ino, pos); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 entry =3D readdir(d); >> =C2=A0 =C2=A0 } while (entry); >> =C2=A0 =C2=A0 closedir(d); >> >> =C2=A0 =C2=A0 return 0; >> } >> >> d_off is equal to telldir value: > > I don't think there is any dispute there.. All I am saying is > > Your diff was this: > > - =C2=A0 =C2=A0vs->dir_pos =3D vs->dent->d_off; > =C2=A0 =C2=A0 =C2=A0vs->dent =3D v9fs_do_readdir(s, vs->fidp->dir); > + =C2=A0 =C2=A0vs->dir_pos =3D v9fs_do_telldir(s, vs->fidp->dir); > > > It should be: > - =C2=A0 =C2=A0vs->dir_pos =3D vs->dent->d_off; > + =C2=A0 vs->dir_pos =3D v9fs_do_telldir(s, vs->fidp->dir); > =C2=A0 =C2=A0 =C2=A0vs->dent =3D v9fs_do_readdir(s, vs->fidp->dir); > Fully agree. I just interpreted the comment in the manual page so that d_off would be the offset to the next dirent while in reality it's the offset of the dirent in question. So telldir returns the same offset as d_off. > >> $ ./dirent / >> name tmp d_off 206002973 ino 56 pos 206002973 >> name root d_off 224116791 ino 521217 pos 224116791 >> name vmlinuz.old d_off 255549115 ino 17 pos 255549115 >> name dev d_off 378658993 ino 374625 pos 378658993 >> >> I'll send an updated patch. >> >>> BTW, Thanks for making VirtFS generic to all POSIX systems. >>> >>> Thanks, >>> JV. >>> >>>> =C2=A0 =C2=A0 =C2=A0v9fs_read_post_readdir(s, vs, err); >>>> =C2=A0 =C2=A0 =C2=A0return; >>>> =C2=A0out: >>>> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c >>>> index 7ddf612..0a74781 100644 >>>> --- a/hw/virtio-pci.c >>>> +++ b/hw/virtio-pci.c >>>> @@ -102,7 +102,7 @@ typedef struct { >>>> =C2=A0 =C2=A0 =C2=A0BlockConf block; >>>> =C2=A0 =C2=A0 =C2=A0NICConf nic; >>>> =C2=A0 =C2=A0 =C2=A0uint32_t host_features; >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0 =C2=A0 =C2=A0V9fsConf fsconf; >>>> =C2=A0#endif >>>> =C2=A0 =C2=A0 =C2=A0/* Max. number of ports we can have for a the virt= io-serial device */ >>>> @@ -642,7 +642,7 @@ static int virtio_balloon_init_pci(PCIDevice *pci_= dev) >>>> =C2=A0 =C2=A0 =C2=A0return 0; >>>> =C2=A0} >>>> >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0static int virtio_9p_init_pci(PCIDevice *pci_dev) >>>> =C2=A0{ >>>> =C2=A0 =C2=A0 =C2=A0VirtIOPCIProxy *proxy =3D DO_UPCAST(VirtIOPCIProxy= , pci_dev, pci_dev); >>>> @@ -713,7 +713,7 @@ static PCIDeviceInfo virtio_info[] =3D { >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}, >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.qdev.reset =3D virtio_pci_reset, >>>> =C2=A0 =C2=A0 =C2=A0},{ >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.qdev.name =3D "virtio-9p-pci", >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.qdev.size =3D sizeof(VirtIOPCIProxy= ), >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.init =C2=A0 =C2=A0 =C2=A0=3D virtio= _9p_init_pci, >>>> diff --git a/hw/virtio.h b/hw/virtio.h >>>> index e4306cd..e77af13 100644 >>>> --- a/hw/virtio.h >>>> +++ b/hw/virtio.h >>>> @@ -20,7 +20,7 @@ >>>> =C2=A0#include "sysemu.h" >>>> =C2=A0#include "block_int.h" >>>> =C2=A0#include "event_notifier.h" >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0#include "9p.h" >>>> =C2=A0#endif >>>> >>>> @@ -188,7 +188,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, >>>> BlockConf *conf); >>>> =C2=A0VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf); >>>> =C2=A0VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_= nr_ports); >>>> =C2=A0VirtIODevice *virtio_balloon_init(DeviceState *dev); >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf); >>>> =C2=A0#endif >>>> >>>> diff --git a/qemu-config.c b/qemu-config.c >>>> index d500885..78e80e3 100644 >>>> --- a/qemu-config.c >>>> +++ b/qemu-config.c >>>> @@ -151,7 +151,7 @@ QemuOptsList qemu_chardev_opts =3D { >>>> =C2=A0 =C2=A0 =C2=A0}, >>>> =C2=A0}; >>>> >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0QemuOptsList qemu_fsdev_opts =3D { >>>> =C2=A0 =C2=A0 =C2=A0.name =3D "fsdev", >>>> =C2=A0 =C2=A0 =C2=A0.implied_opt_name =3D "fstype", >>>> @@ -169,7 +169,7 @@ QemuOptsList qemu_fsdev_opts =3D { >>>> =C2=A0}; >>>> =C2=A0#endif >>>> >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0QemuOptsList qemu_virtfs_opts =3D { >>>> =C2=A0 =C2=A0 =C2=A0.name =3D "virtfs", >>>> =C2=A0 =C2=A0 =C2=A0.implied_opt_name =3D "fstype", >>>> diff --git a/qemu-config.h b/qemu-config.h >>>> index dca69d4..5376935 100644 >>>> --- a/qemu-config.h >>>> +++ b/qemu-config.h >>>> @@ -3,7 +3,7 @@ >>>> >>>> =C2=A0extern QemuOptsList qemu_drive_opts; >>>> =C2=A0extern QemuOptsList qemu_chardev_opts; >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0extern QemuOptsList qemu_fsdev_opts; >>>> =C2=A0extern QemuOptsList qemu_virtfs_opts; >>>> =C2=A0#endif >>>> diff --git a/qemu-options.hx b/qemu-options.hx >>>> index 03e95fd..34ed806 100644 >>>> --- a/qemu-options.hx >>>> +++ b/qemu-options.hx >>>> @@ -475,7 +475,7 @@ possible drivers and properties, use @code{-device= ?} and >>>> =C2=A0@code{-device @var{driver},?}. >>>> =C2=A0ETEXI >>>> >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0DEFHEADING(File system options:) >>>> >>>> =C2=A0DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev, >>>> @@ -499,7 +499,7 @@ Options to each backend are described below. >>>> >>>> =C2=A0Create a file-system-"device" for local-filesystem. >>>> >>>> -@option{local} is only available on Linux. >>>> +@option{local} is only available on POSIX systems. >>>> >>>> =C2=A0@option{path} specifies the path to be exported. @option{path} i= s required. >>>> >>>> @@ -507,7 +507,7 @@ Create a file-system-"device" for local-filesystem= . >>>> =C2=A0ETEXI >>>> =C2=A0#endif >>>> >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0DEFHEADING(Virtual File system pass-through options:) >>>> >>>> =C2=A0DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs, >>>> @@ -531,7 +531,7 @@ Options to each backend are described below. >>>> >>>> =C2=A0Create a Virtual file-system-pass through for local-filesystem. >>>> >>>> -@option{local} is only available on Linux. >>>> +@option{local} is only available on POSIX systems. >>>> >>>> =C2=A0@option{path} specifies the path to be exported. @option{path} i= s required. >>>> >>>> diff --git a/vl.c b/vl.c >>>> index d77b47c..d5c1e34 100644 >>>> --- a/vl.c >>>> +++ b/vl.c >>>> @@ -149,7 +149,7 @@ int main(int argc, char **argv) >>>> =C2=A0#include "qemu-option.h" >>>> =C2=A0#include "qemu-config.h" >>>> =C2=A0#include "qemu-objects.h" >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0#include "fsdev/qemu-fsdev.h" >>>> =C2=A0#endif >>>> >>>> @@ -2314,7 +2314,7 @@ static int chardev_init_func(QemuOpts *opts, voi= d *opaque) >>>> =C2=A0 =C2=A0 =C2=A0return 0; >>>> =C2=A0} >>>> >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0static int fsdev_init_func(QemuOpts *opts, void *opaque) >>>> =C2=A0{ >>>> =C2=A0 =C2=A0 =C2=A0int ret; >>>> @@ -3090,7 +3090,7 @@ int main(int argc, char **argv, char **envp) >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0exit(1); >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break; >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case QEMU_OPTION_fsdev= : >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0opts =3D= qemu_opts_parse(&qemu_fsdev_opts, optarg, 1); >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!opt= s) { >>>> @@ -3513,7 +3513,7 @@ int main(int argc, char **argv, char **envp) >>>> >>>> =C2=A0 =C2=A0 =C2=A0if (qemu_opts_foreach(&qemu_chardev_opts, chardev_= init_func, NULL, 1) !=3D 0) >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0exit(1); >>>> -#ifdef CONFIG_LINUX >>>> +#ifdef CONFIG_POSIX >>>> =C2=A0 =C2=A0 =C2=A0if (qemu_opts_foreach(&qemu_fsdev_opts, fsdev_init= _func, NULL, 1) !=3D 0) { >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0exit(1); >>>> =C2=A0 =C2=A0 =C2=A0} >>> >>> > > >