* [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
@ 2017-08-09 7:17 Cornelia Huck
2017-08-09 8:23 ` Thomas Huth
0 siblings, 1 reply; 14+ messages in thread
From: Cornelia Huck @ 2017-08-09 7:17 UTC (permalink / raw)
To: aneesh.kumar, groug; +Cc: borntraeger, agraf, thuth, qemu-devel, Cornelia Huck
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 <cohuck@redhat.com>
---
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(-)
diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
index 51191b77df..e4c5236ceb 100644
--- a/default-configs/s390x-softmmu.mak
+++ b/default-configs/s390x-softmmu.mak
@@ -8,3 +8,4 @@ CONFIG_S390_FLIC=y
CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
CONFIG_VFIO_CCW=$(CONFIG_LINUX)
CONFIG_WDT_DIAG288=y
+CONFIG_VIRTIO_CCW=y
diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
index 659df6e187..3d157add31 100644
--- a/fsdev/Makefile.objs
+++ b/fsdev/Makefile.objs
@@ -1,10 +1,7 @@
-ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
# Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
-# only pull in the actual virtio-9p device if we also enabled virtio.
-common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
-else
-common-obj-y = qemu-fsdev-dummy.o
-endif
+# only pull in the actual virtio-9p device if we also enabled a virtio backend.
+common-obj-$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))= qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
+common-obj-$(call lnot,$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))) = qemu-fsdev-dummy.o
common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
# Toplevel always builds this; targets without virtio will put it in
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index a2c61f6b09..335f26b65e 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -1,4 +1,4 @@
-devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
+devices-dirs-$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW))) += 9pfs/
devices-dirs-$(CONFIG_SOFTMMU) += acpi/
devices-dirs-$(CONFIG_SOFTMMU) += adc/
devices-dirs-$(CONFIG_SOFTMMU) += audio/
--
2.13.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 7:17 [Qemu-devel] [PATCH v2] 9pfs: fix dependencies Cornelia Huck
@ 2017-08-09 8:23 ` Thomas Huth
2017-08-09 8:27 ` Cornelia Huck
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2017-08-09 8:23 UTC (permalink / raw)
To: Cornelia Huck, aneesh.kumar, groug; +Cc: borntraeger, agraf, qemu-devel
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 <cohuck@redhat.com>
> ---
>
> 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(-)
>
> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> index 51191b77df..e4c5236ceb 100644
> --- a/default-configs/s390x-softmmu.mak
> +++ b/default-configs/s390x-softmmu.mak
> @@ -8,3 +8,4 @@ CONFIG_S390_FLIC=y
> CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
> CONFIG_VFIO_CCW=$(CONFIG_LINUX)
> CONFIG_WDT_DIAG288=y
> +CONFIG_VIRTIO_CCW=y
> diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
> index 659df6e187..3d157add31 100644
> --- a/fsdev/Makefile.objs
> +++ b/fsdev/Makefile.objs
> @@ -1,10 +1,7 @@
> -ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
> # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
> -# only pull in the actual virtio-9p device if we also enabled virtio.
> -common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> -else
> -common-obj-y = qemu-fsdev-dummy.o
> -endif
> +# only pull in the actual virtio-9p device if we also enabled a virtio backend.
> +common-obj-$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))= qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> +common-obj-$(call lnot,$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))) = qemu-fsdev-dummy.o
> common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
>
> # Toplevel always builds this; targets without virtio will put it in
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index a2c61f6b09..335f26b65e 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -1,4 +1,4 @@
> -devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
> +devices-dirs-$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW))) += 9pfs/
> devices-dirs-$(CONFIG_SOFTMMU) += acpi/
> devices-dirs-$(CONFIG_SOFTMMU) += adc/
> devices-dirs-$(CONFIG_SOFTMMU) += audio/
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 ;-)
Thomas
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 8:23 ` Thomas Huth
@ 2017-08-09 8:27 ` Cornelia Huck
2017-08-09 9:07 ` Thomas Huth
2017-08-09 9:47 ` Greg Kurz
0 siblings, 2 replies; 14+ messages in thread
From: Cornelia Huck @ 2017-08-09 8:27 UTC (permalink / raw)
To: Thomas Huth; +Cc: aneesh.kumar, groug, borntraeger, agraf, qemu-devel
On Wed, 9 Aug 2017 10:23:04 +0200
Thomas Huth <thuth@redhat.com> 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 <cohuck@redhat.com>
> > ---
> >
> > 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(-)
> >
> > diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> > index 51191b77df..e4c5236ceb 100644
> > --- a/default-configs/s390x-softmmu.mak
> > +++ b/default-configs/s390x-softmmu.mak
> > @@ -8,3 +8,4 @@ CONFIG_S390_FLIC=y
> > CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
> > CONFIG_VFIO_CCW=$(CONFIG_LINUX)
> > CONFIG_WDT_DIAG288=y
> > +CONFIG_VIRTIO_CCW=y
> > diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
> > index 659df6e187..3d157add31 100644
> > --- a/fsdev/Makefile.objs
> > +++ b/fsdev/Makefile.objs
> > @@ -1,10 +1,7 @@
> > -ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
> > # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
> > -# only pull in the actual virtio-9p device if we also enabled virtio.
> > -common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> > -else
> > -common-obj-y = qemu-fsdev-dummy.o
> > -endif
> > +# only pull in the actual virtio-9p device if we also enabled a virtio backend.
> > +common-obj-$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))= qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> > +common-obj-$(call lnot,$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))) = qemu-fsdev-dummy.o
> > common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
> >
> > # Toplevel always builds this; targets without virtio will put it in
> > diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> > index a2c61f6b09..335f26b65e 100644
> > --- a/hw/Makefile.objs
> > +++ b/hw/Makefile.objs
> > @@ -1,4 +1,4 @@
> > -devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
> > +devices-dirs-$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW))) += 9pfs/
> > devices-dirs-$(CONFIG_SOFTMMU) += acpi/
> > devices-dirs-$(CONFIG_SOFTMMU) += adc/
> > devices-dirs-$(CONFIG_SOFTMMU) += audio/
>
> 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...
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 8:27 ` Cornelia Huck
@ 2017-08-09 9:07 ` Thomas Huth
2017-08-09 9:13 ` Cornelia Huck
` (3 more replies)
2017-08-09 9:47 ` Greg Kurz
1 sibling, 4 replies; 14+ messages in thread
From: Thomas Huth @ 2017-08-09 9:07 UTC (permalink / raw)
To: Cornelia Huck; +Cc: aneesh.kumar, groug, borntraeger, agraf, qemu-devel
On 09.08.2017 10:27, Cornelia Huck wrote:
> On Wed, 9 Aug 2017 10:23:04 +0200
> Thomas Huth <thuth@redhat.com> 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 <cohuck@redhat.com>
>>> ---
>>>
>>> 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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 9:07 ` Thomas Huth
@ 2017-08-09 9:13 ` Cornelia Huck
2017-08-09 9:24 ` Daniel P. Berrange
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Cornelia Huck @ 2017-08-09 9:13 UTC (permalink / raw)
To: Thomas Huth; +Cc: aneesh.kumar, groug, borntraeger, agraf, qemu-devel
On Wed, 9 Aug 2017 11:07:38 +0200
Thomas Huth <thuth@redhat.com> wrote:
> On 09.08.2017 10:27, Cornelia Huck wrote:
> > On Wed, 9 Aug 2017 10:23:04 +0200
> > Thomas Huth <thuth@redhat.com> 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 <cohuck@redhat.com>
> >>> ---
> >>>
> >>> 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.
Yes, I'd prefer if somebody with a virtio-mmio setup could chime in.
Given the current Makefiles, this cannot have worked for !pci anyway...
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 9:07 ` Thomas Huth
2017-08-09 9:13 ` Cornelia Huck
@ 2017-08-09 9:24 ` Daniel P. Berrange
2017-08-09 9:43 ` Cornelia Huck
2017-08-09 9:44 ` Peter Maydell
2017-08-09 9:47 ` Peter Maydell
3 siblings, 1 reply; 14+ messages in thread
From: Daniel P. Berrange @ 2017-08-09 9:24 UTC (permalink / raw)
To: Thomas Huth
Cc: Cornelia Huck, qemu-devel, borntraeger, agraf, aneesh.kumar,
groug
On Wed, Aug 09, 2017 at 11:07:38AM +0200, Thomas Huth wrote:
> On 09.08.2017 10:27, Cornelia Huck wrote:
> > On Wed, 9 Aug 2017 10:23:04 +0200
> > Thomas Huth <thuth@redhat.com> 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 <cohuck@redhat.com>
> >>> ---
> >>>
> >>> 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.
qemu-system-arm supports virtio-mmio so you can use that to test it
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 9:24 ` Daniel P. Berrange
@ 2017-08-09 9:43 ` Cornelia Huck
0 siblings, 0 replies; 14+ messages in thread
From: Cornelia Huck @ 2017-08-09 9:43 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Thomas Huth, qemu-devel, borntraeger, agraf, aneesh.kumar, groug
On Wed, 9 Aug 2017 10:24:13 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:
> On Wed, Aug 09, 2017 at 11:07:38AM +0200, Thomas Huth wrote:
> > On 09.08.2017 10:27, Cornelia Huck wrote:
> > > On Wed, 9 Aug 2017 10:23:04 +0200
> > > Thomas Huth <thuth@redhat.com> 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 <cohuck@redhat.com>
> > >>> ---
> > >>>
> > >>> 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.
>
> qemu-system-arm supports virtio-mmio so you can use that to test it
Hm, the default config for arm enables CONFIG_PCI, so machines using
virtio-mmio and 9p would be broken with that patch... should we rather
depend on PCI || VIRTIO_CCW?
(Any arches not enabling PCI that use virtio-mmio? Or is arm the only
user of virtio-mmio?)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 9:07 ` Thomas Huth
2017-08-09 9:13 ` Cornelia Huck
2017-08-09 9:24 ` Daniel P. Berrange
@ 2017-08-09 9:44 ` Peter Maydell
2017-08-09 9:47 ` Peter Maydell
3 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2017-08-09 9:44 UTC (permalink / raw)
To: Thomas Huth
Cc: Cornelia Huck, QEMU Developers, Christian Borntraeger,
Alexander Graf, Aneesh Kumar K.V, Greg Kurz
On 9 August 2017 at 10:07, Thomas Huth <thuth@redhat.com> wrote:
> While virtio-pci and virtio-ccw seem to require separate dedicated
> devices (e.g. virtio-9p-pci and virtio-9p-ccw) for everything,
You don't *have* to use the dedicated virtio-foo-pci device;
if you like you can manually plug together the virtio-pci
transport and the virtio-foo-device backend yourself. The
'fused' device is just for convenience and compatibility
with existing command lines. There is no fused device for
virtio-mmio because the board itself creates the transport
devices so the user only needs to create the backends
(which then auto-plug into the virtio bus).
thanks
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 9:07 ` Thomas Huth
` (2 preceding siblings ...)
2017-08-09 9:44 ` Peter Maydell
@ 2017-08-09 9:47 ` Peter Maydell
2017-08-09 9:52 ` Cornelia Huck
3 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2017-08-09 9:47 UTC (permalink / raw)
To: Thomas Huth
Cc: Cornelia Huck, QEMU Developers, Christian Borntraeger,
Alexander Graf, Aneesh Kumar K.V, Greg Kurz
On 9 August 2017 at 10:07, Thomas Huth <thuth@redhat.com> wrote:
> 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.
As you say, we already compile the virtio-9p-device that can
plug into any virtio transport. So why not just build it
whenever virtio of any form is enabled? Having it only
build if PCI is also enabled seems very odd: the backend
should not care at all about what transport it is using.
thanks
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 9:47 ` Peter Maydell
@ 2017-08-09 9:52 ` Cornelia Huck
0 siblings, 0 replies; 14+ messages in thread
From: Cornelia Huck @ 2017-08-09 9:52 UTC (permalink / raw)
To: Peter Maydell
Cc: Thomas Huth, QEMU Developers, Christian Borntraeger,
Alexander Graf, Aneesh Kumar K.V, Greg Kurz
On Wed, 9 Aug 2017 10:47:18 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 August 2017 at 10:07, Thomas Huth <thuth@redhat.com> wrote:
> > 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.
>
> As you say, we already compile the virtio-9p-device that can
> plug into any virtio transport. So why not just build it
> whenever virtio of any form is enabled? Having it only
> build if PCI is also enabled seems very odd: the backend
> should not care at all about what transport it is using.
Given the previous discussions, I think just dropping the PCI
dependency is indeed the way to go. I'll send a v3.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 8:27 ` Cornelia Huck
2017-08-09 9:07 ` Thomas Huth
@ 2017-08-09 9:47 ` Greg Kurz
2017-08-09 11:06 ` Cornelia Huck
1 sibling, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2017-08-09 9:47 UTC (permalink / raw)
To: Cornelia Huck
Cc: Thomas Huth, aneesh.kumar, borntraeger, agraf, qemu-devel,
Stefano Stabellini, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 3928 bytes --]
On Wed, 9 Aug 2017 10:27:37 +0200
Cornelia Huck <cohuck@redhat.com> wrote:
> On Wed, 9 Aug 2017 10:23:04 +0200
> Thomas Huth <thuth@redhat.com> 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 <cohuck@redhat.com>
> > > ---
> > >
> > > 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(-)
> > >
> > > diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> > > index 51191b77df..e4c5236ceb 100644
> > > --- a/default-configs/s390x-softmmu.mak
> > > +++ b/default-configs/s390x-softmmu.mak
> > > @@ -8,3 +8,4 @@ CONFIG_S390_FLIC=y
> > > CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
> > > CONFIG_VFIO_CCW=$(CONFIG_LINUX)
> > > CONFIG_WDT_DIAG288=y
> > > +CONFIG_VIRTIO_CCW=y
> > > diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
> > > index 659df6e187..3d157add31 100644
> > > --- a/fsdev/Makefile.objs
> > > +++ b/fsdev/Makefile.objs
> > > @@ -1,10 +1,7 @@
> > > -ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
> > > # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
> > > -# only pull in the actual virtio-9p device if we also enabled virtio.
> > > -common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> > > -else
> > > -common-obj-y = qemu-fsdev-dummy.o
> > > -endif
> > > +# only pull in the actual virtio-9p device if we also enabled a virtio backend.
> > > +common-obj-$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))= qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> > > +common-obj-$(call lnot,$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))) = qemu-fsdev-dummy.o
> > > common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
> > >
> > > # Toplevel always builds this; targets without virtio will put it in
> > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> > > index a2c61f6b09..335f26b65e 100644
> > > --- a/hw/Makefile.objs
> > > +++ b/hw/Makefile.objs
> > > @@ -1,4 +1,4 @@
> > > -devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
> > > +devices-dirs-$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW))) += 9pfs/
> > > devices-dirs-$(CONFIG_SOFTMMU) += acpi/
> > > devices-dirs-$(CONFIG_SOFTMMU) += adc/
> > > devices-dirs-$(CONFIG_SOFTMMU) += audio/
> >
> > 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 ;-)
>
And virtio isn't the only transport for 9p: we also have a Xen backend,
which happen to be built because targets that support Xen also have
CONFIG_PCI I guess.
Cc'ing Stefano and Paolo who had a discussion during the review of
9p Xen backend patches:
https://patchwork.kernel.org/patch/9622325/
> Do all virtio transports have support for 9p, though? I thought it was
> only virtio-pci and virtio-ccw...
Hmm... I don't see any device-specific code in virtio-mmio.. why would it
be different for 9p than for block or net ?
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 9:47 ` Greg Kurz
@ 2017-08-09 11:06 ` Cornelia Huck
2017-08-09 12:10 ` Greg Kurz
0 siblings, 1 reply; 14+ messages in thread
From: Cornelia Huck @ 2017-08-09 11:06 UTC (permalink / raw)
To: Greg Kurz
Cc: Thomas Huth, aneesh.kumar, borntraeger, agraf, qemu-devel,
Stefano Stabellini, Paolo Bonzini
On Wed, 9 Aug 2017 11:47:05 +0200
Greg Kurz <groug@kaod.org> wrote:
> On Wed, 9 Aug 2017 10:27:37 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
> > On Wed, 9 Aug 2017 10:23:04 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> > > 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 ;-)
> >
>
> And virtio isn't the only transport for 9p: we also have a Xen backend,
> which happen to be built because targets that support Xen also have
> CONFIG_PCI I guess.
Only if they also have virtio enabled, no?
Should the condition be VIRTFS && (VIRTIO || XEN), then?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 11:06 ` Cornelia Huck
@ 2017-08-09 12:10 ` Greg Kurz
2017-08-09 16:02 ` Cornelia Huck
0 siblings, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2017-08-09 12:10 UTC (permalink / raw)
To: Cornelia Huck
Cc: Thomas Huth, aneesh.kumar, borntraeger, agraf, qemu-devel,
Stefano Stabellini, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1243 bytes --]
On Wed, 9 Aug 2017 13:06:14 +0200
Cornelia Huck <cohuck@redhat.com> wrote:
> On Wed, 9 Aug 2017 11:47:05 +0200
> Greg Kurz <groug@kaod.org> wrote:
>
> > On Wed, 9 Aug 2017 10:27:37 +0200
> > Cornelia Huck <cohuck@redhat.com> wrote:
> >
> > > On Wed, 9 Aug 2017 10:23:04 +0200
> > > Thomas Huth <thuth@redhat.com> wrote:
>
> > > > 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 ;-)
> > >
> >
> > And virtio isn't the only transport for 9p: we also have a Xen backend,
> > which happen to be built because targets that support Xen also have
> > CONFIG_PCI I guess.
>
> Only if they also have virtio enabled, no?
>
Yes, you're right. This is actually the case for i386 and x86_64 targets,
which seem to be the only that support Xen.
> Should the condition be VIRTFS && (VIRTIO || XEN), then?
That's what I was beginning to think as well :)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies
2017-08-09 12:10 ` Greg Kurz
@ 2017-08-09 16:02 ` Cornelia Huck
0 siblings, 0 replies; 14+ messages in thread
From: Cornelia Huck @ 2017-08-09 16:02 UTC (permalink / raw)
To: Greg Kurz
Cc: Thomas Huth, aneesh.kumar, borntraeger, agraf, qemu-devel,
Stefano Stabellini, Paolo Bonzini
On Wed, 9 Aug 2017 14:10:01 +0200
Greg Kurz <groug@kaod.org> wrote:
> On Wed, 9 Aug 2017 13:06:14 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> > Should the condition be VIRTFS && (VIRTIO || XEN), then?
>
> That's what I was beginning to think as well :)
OK, here's what should work:
- fsdev/Makefile.objs needs to check for VIRTFS && (VIRTIO || XEN)
- hw/Makefile.objs needs to check for VIRTFS && (VIRTIO || XEN) before
building hw/9pfs/
- hw/9pfs/Makefile.objs needs a new VIRTIO check for the virtio device
I'm preparing a patch.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-08-09 16:02 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-09 7:17 [Qemu-devel] [PATCH v2] 9pfs: fix dependencies Cornelia Huck
2017-08-09 8:23 ` Thomas Huth
2017-08-09 8:27 ` Cornelia Huck
2017-08-09 9:07 ` Thomas Huth
2017-08-09 9:13 ` Cornelia Huck
2017-08-09 9:24 ` Daniel P. Berrange
2017-08-09 9:43 ` Cornelia Huck
2017-08-09 9:44 ` Peter Maydell
2017-08-09 9:47 ` Peter Maydell
2017-08-09 9:52 ` Cornelia Huck
2017-08-09 9:47 ` Greg Kurz
2017-08-09 11:06 ` Cornelia Huck
2017-08-09 12:10 ` Greg Kurz
2017-08-09 16:02 ` Cornelia Huck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).