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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

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).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]