[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 4/4] fsdev: QMP interface for throttling
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH v4 4/4] fsdev: QMP interface for throttling |
Date: |
Fri, 9 Jun 2017 11:07:41 +0200 |
On Fri, 9 Jun 2017 10:31:58 +0200
Pradeep Jagadeesh <address@hidden> wrote:
> 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.
>
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
> >>>
> >>
> >>
> >
>
pgpC6oI5P2yCC.pgp
Description: OpenPGP digital signature