On Fri, 9 Jun 2017 10:31:58 +0200 Pradeep Jagadeesh <pradeep.jagade...@huawei.com> wrote:
> On 6/8/2017 6:49 PM, Greg Kurz wrote: > > On Thu, 18 May 2017 15:30:06 +0200 > > Pradeep Jagadeesh <pradeep.jagade...@huawei.com> wrote: > > > >> On 5/17/2017 7:09 PM, Eric Blake wrote: > >>> On 05/17/2017 11:29 AM, Greg Kurz wrote: > >>> > >>>>> > >>>>> First point: is fsdev a Linux-only feature, or can it be compiled on > >>>>> BSD? If it is Linux-only, then compiling a stub for Windows will still > >>>>> leave BSD broken, and your #ifdef is wrong. Fixing compilation on mingw > >>>>> is nice, but not the only platform to worry about. > >>>>> > >>>> > >>>> fsdev compilation currently depends on CONFIG_VIRTFS which is a > >>>> Linux-only > >>>> feature for the moment. There was a tentative to support it on Windows > >>>> hosts > >>>> two years ago but it stayed at the RFC stage. > >>>> > >>>> But even on Linux hosts, the current fsdev implementation also depends on > >>>> the target supporting PCI and VIRTIO. We have a fsdev/qemu-fsdev-dummy.c > >>>> file to put stubs so that we don't pull all the code for such targets. > >>>> > >>>> Maybe this could be reused for the above stubs as well ? > >>> > >>> That helps. The stub should live in qemu-fsdev-dummy.c (where it > >>> shouldn't need any #ifdef, because that file is only compiled when the > >>> condition is false), and... > >>> > >>>> > >>>>> Second point: if fsdev is indeed a platform-specific feature, then we > >>>>> don't want to advertise the QMP commands that are useless when running > >>>>> on a platform that doesn't support it. Anywhere you have to add a stub > >>>>> for compilation means you ALSO need to patch monitor.c to unregister the > >>>>> command from being advertised as a valid QMP command. (If you used > >>>>> #ifdef __LINUX__ to guard the working version, and #ifndef __LINUX__ to > >>>>> declare the stub, then the monitor.c needs an #ifndef section within > >>>>> qmp_unregister_commands_hack() to tell QMP to not advertise the stubs.) > >>>>> > >>> > >>> monitor.c should wrap the unregister under #ifndef CONFIG_VIRTFS (rather > >>> than a particular platform name). > >> I did unregister the functions as mentioned above in monitor.c. But it > >> does not address the issue when I run "make docker-test-mingw@fedora". > > > > What issue is this ? > > > > I cannot even run this with your unmodified series (please note that I > > always build out-of-tree). > The issue is when I cross build the qemu, I was facing the compilation > issue. Even it failed in patchew. So, I had to add the dummy functions > in monitor.c and qmp.c. > I see no dummy functions in monitor.c ... As said above, I cannot even reach that point with my out-of-tree build setup: GEN qmp-commands.h /tmp/qemu-test/src/qapi-schema.json:85: No such file or directory: qapi/fsdev.json Makefile:438: recipe for target 'qmp-commands.h' failed Please fix that. > -Pradeep > >> I think the VIRTFS is still enabled. > >> Only having dummy functions even in qmp.c addresses the issue. > >> > >> Regards, > >> Pradeep > >>> > >> > >> > > >
pgp8LixlXYvmd.pgp
Description: OpenPGP digital signature
