Re: [Qemu-devel] [PATCH v4 4/4] fsdev: QMP interface for throttling

[Pradeep Jagadeesh]

On 6/8/2017 6:49 PM, Greg Kurz wrote:
> On Thu, 18 May 2017 15:30:06 +0200
> Pradeep Jagadeesh <address@hidden> 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 address@hidden".
>
> 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.

-Pradeep
> > I think the VIRTFS is still enabled.
> > Only having dummy functions even in qmp.c addresses the issue.
> >
> > Regards,
> > Pradeep
> > >