Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies

[Thomas Huth]

On 09.08.2017 10:27, Cornelia Huck wrote:
> On Wed, 9 Aug 2017 10:23:04 +0200
> Thomas Huth <address@hidden> wrote:
> 
>> On 09.08.2017 09:17, Cornelia Huck wrote:
>>> Nothing in fsdev/ or hw/9pfs/ depends on pci; it should rather depend
>>> on CONFIG_VIRTFS and on the presence of an appropriate virtio transport
>>> device.
>>>
>>> Let's introduce CONFIG_VIRTIO_CCW to cover s390x and check for
>>> CONFIG_VIRTFS && (CONFIG_VIRTIO_PCI || CONFIG_VIRTIO_CCW).
>>>
>>> Signed-off-by: Cornelia Huck <address@hidden>
>>> ---
>>>
>>> Changes v1->v2: drop extraneous spaces, fix build on cris
>>>
>>> ---
>>>  default-configs/s390x-softmmu.mak | 1 +
>>>  fsdev/Makefile.objs               | 9 +++------
>>>  hw/Makefile.objs                  | 2 +-
>>>  3 files changed, 5 insertions(+), 7 deletions(-)
[...]
>>
>> Patch should be fine now, I think...
>>
>> But thinking about this again, I wonder whether it would be enough to
>> simply check for CONFIG_VIRTIO=y here instead. CONFIG_VIRTIO=y should be
>> sufficient to assert that there is also at least one kind of virtio
>> transport available, right?
>> Otherwise this will look really horrible as soon as somebody also tries
>> to add support for virtio-mmio here later ;-)
> 
> Do all virtio transports have support for 9p, though? I thought it was
> only virtio-pci and virtio-ccw...

While virtio-pci and virtio-ccw seem to require separate dedicated
devices (e.g. virtio-9p-pci and virtio-9p-ccw) for everything,
virtio-mmio seems to work different. As far as I can see, there are no
dedicated virtio-xxx-mmio devices in the code at all. According to
https://www.redhat.com/archives/libvir-list/2013-August/msg00009.html
you simply have to use virtio-xxx-device here instead. And a
virtio-9p-device is available. So theoretically, the 9p code should work
with virtio-mmio, too, or is there a problem that I did not see yet?

Anyway, we likely should not blindly enable this, so unless somebody has
a setup to test it, we should go with your current patch instead, I think.

 Thomas