[Qemu-devel] [PULL] virtio-ccw: dataplane enablement

[Qemu-devel] [PULL] virtio-ccw: dataplane enablement

[Cornelia Huck]

The following changes since commit c3cb8e77804313e1be99b5f28a34a346736707a5:

  ioport: remove LITTLE_ENDIAN mark for portio (2013-07-12 14:37:47 -0500)

are available in the git repository at:

  git://github.com/cohuck/qemu virtio-ccw-upstr

for you to fetch changes up to bf72d89f0a8fb27a6bfde1a689690fd577227762:

  virtio-ccw: Enable x-data-plane for virtio-ccw-blk (2013-07-15 17:39:04 +0200)

----------------------------------------------------------------
Dominik Dingel (1):
  virtio-ccw: Enable x-data-plane for virtio-ccw-blk

 hw/s390x/virtio-ccw.c |    3 +++
 1 file changed, 3 insertions(+)

-- 
1.7.9.5

[Qemu-devel] [PULL] virtio-ccw: Enable x-data-plane for virtio-ccw-blk

[Cornelia Huck]

From: Dominik Dingel <dingel@linux.vnet.ibm.com>

Add property x-data-plane to virtio-ccw-blk devices.

Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index e744957..8835bd4 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1031,6 +1031,9 @@ static Property virtio_ccw_blk_properties[] = {
     DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, blk),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+    DEFINE_PROP_BIT("x-data-plane", VirtIOBlkCcw, blk.data_plane, 0, false),
+#endif
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.7.9.5

Re: [Qemu-devel] [PULL] virtio-ccw: dataplane enablement

[Anthony Liguori]

Cornelia Huck <cornelia.huck@de.ibm.com> writes:

> The following changes since commit c3cb8e77804313e1be99b5f28a34a346736707a5:
>
>   ioport: remove LITTLE_ENDIAN mark for portio (2013-07-12 14:37:47 -0500)
>
> are available in the git repository at:
>
>   git://github.com/cohuck/qemu virtio-ccw-upstr
>
> for you to fetch changes up to bf72d89f0a8fb27a6bfde1a689690fd577227762:
>
>   virtio-ccw: Enable x-data-plane for virtio-ccw-blk (2013-07-15
>   17:39:04 +0200)

This is v1?  This was just posted on Thursday?  That's a bit too quick
for a pull request.

I've seen no review from Stefan or Paolo and I would think that given
the work that's been done already, it would be counterproductive to
enable x-data-plane for another backend.

Paolo/Stefan, what do you guys think?

Regards,

Anthony Liguori


>
> ----------------------------------------------------------------
> Dominik Dingel (1):
>   virtio-ccw: Enable x-data-plane for virtio-ccw-blk
>
>  hw/s390x/virtio-ccw.c |    3 +++
>  1 file changed, 3 insertions(+)
>
> -- 
> 1.7.9.5

Re: [Qemu-devel] [PULL] virtio-ccw: dataplane enablement

[Paolo Bonzini]

Il 15/07/2013 18:52, Anthony Liguori ha scritto:
> Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> 
>> The following changes since commit c3cb8e77804313e1be99b5f28a34a346736707a5:
>>
>>   ioport: remove LITTLE_ENDIAN mark for portio (2013-07-12 14:37:47 -0500)
>>
>> are available in the git repository at:
>>
>>   git://github.com/cohuck/qemu virtio-ccw-upstr
>>
>> for you to fetch changes up to bf72d89f0a8fb27a6bfde1a689690fd577227762:
>>
>>   virtio-ccw: Enable x-data-plane for virtio-ccw-blk (2013-07-15
>>   17:39:04 +0200)
> 
> This is v1?  This was just posted on Thursday?  That's a bit too quick
> for a pull request.
> 
> I've seen no review from Stefan or Paolo and I would think that given
> the work that's been done already, it would be counterproductive to
> enable x-data-plane for another backend.
> 
> Paolo/Stefan, what do you guys think?

Considering what the patch looks like, I don't think it's a huge
problem...  In fact, perhaps x-data-plane could be even added to
DEFINE_VIRTIO_BLK_PROPERTIES.  This would make it clear that support for
non-ioeventfd hosts (including TCG) is one of the things to do to make
data plane the default.

Paolo

Re: [Qemu-devel] [PULL] virtio-ccw: dataplane enablement

[Stefan Hajnoczi]

On Mon, Jul 15, 2013 at 07:15:55PM +0200, Paolo Bonzini wrote:
> Il 15/07/2013 18:52, Anthony Liguori ha scritto:
> > Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > 
> >> The following changes since commit c3cb8e77804313e1be99b5f28a34a346736707a5:
> >>
> >>   ioport: remove LITTLE_ENDIAN mark for portio (2013-07-12 14:37:47 -0500)
> >>
> >> are available in the git repository at:
> >>
> >>   git://github.com/cohuck/qemu virtio-ccw-upstr
> >>
> >> for you to fetch changes up to bf72d89f0a8fb27a6bfde1a689690fd577227762:
> >>
> >>   virtio-ccw: Enable x-data-plane for virtio-ccw-blk (2013-07-15
> >>   17:39:04 +0200)
> > 
> > This is v1?  This was just posted on Thursday?  That's a bit too quick
> > for a pull request.
> > 
> > I've seen no review from Stefan or Paolo and I would think that given
> > the work that's been done already, it would be counterproductive to
> > enable x-data-plane for another backend.
> > 
> > Paolo/Stefan, what do you guys think?
> 
> Considering what the patch looks like, I don't think it's a huge
> problem...  In fact, perhaps x-data-plane could be even added to
> DEFINE_VIRTIO_BLK_PROPERTIES.  This would make it clear that support for
> non-ioeventfd hosts (including TCG) is one of the things to do to make
> data plane the default.

I agree.  Since this patch is so minimal it will not add extra work as
we reintegrate dataplane into core QEMU.

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PULL] virtio-ccw: dataplane enablement

[Cornelia Huck]

On Tue, 16 Jul 2013 15:10:41 +0800
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Mon, Jul 15, 2013 at 07:15:55PM +0200, Paolo Bonzini wrote:
> > Il 15/07/2013 18:52, Anthony Liguori ha scritto:
> > > Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > > 
> > >> The following changes since commit c3cb8e77804313e1be99b5f28a34a346736707a5:
> > >>
> > >>   ioport: remove LITTLE_ENDIAN mark for portio (2013-07-12 14:37:47 -0500)
> > >>
> > >> are available in the git repository at:
> > >>
> > >>   git://github.com/cohuck/qemu virtio-ccw-upstr
> > >>
> > >> for you to fetch changes up to bf72d89f0a8fb27a6bfde1a689690fd577227762:
> > >>
> > >>   virtio-ccw: Enable x-data-plane for virtio-ccw-blk (2013-07-15
> > >>   17:39:04 +0200)
> > > 
> > > This is v1?  This was just posted on Thursday?  That's a bit too quick
> > > for a pull request.
> > > 
> > > I've seen no review from Stefan or Paolo and I would think that given
> > > the work that's been done already, it would be counterproductive to
> > > enable x-data-plane for another backend.
> > > 
> > > Paolo/Stefan, what do you guys think?
> > 
> > Considering what the patch looks like, I don't think it's a huge
> > problem...  In fact, perhaps x-data-plane could be even added to
> > DEFINE_VIRTIO_BLK_PROPERTIES.  This would make it clear that support for
> > non-ioeventfd hosts (including TCG) is one of the things to do to make
> > data plane the default.
> 
> I agree.  Since this patch is so minimal it will not add extra work as
> we reintegrate dataplane into core QEMU.
> 
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
Thank you.

Any further guidelines on when to ask for acks from you guys? The mode
I've been operating in is mostly "if virtio-pci provides this feature,
it shouldn't be half bad and we want it in virtio-ccw as well in order
to keep things consistent and get more test coverage".

(dataplane has been enabled in our internal testing for some time now,
I just had asked Dominik to re-do his patch with a proper description
as we had some code churn in our internal repository.)

Re: [Qemu-devel] [PULL] virtio-ccw: dataplane enablement

[Paolo Bonzini]

Il 16/07/2013 10:54, Cornelia Huck ha scritto:
>> > Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
>> > 
> Thank you.
> 
> Any further guidelines on when to ask for acks from you guys? The mode
> I've been operating in is mostly "if virtio-pci provides this feature,
> it shouldn't be half bad and we want it in virtio-ccw as well in order
> to keep things consistent and get more test coverage".

I agree with these guidelines, in fact neither I nor Stefan saw a
problem with your patch. :)

Paolo

Re: [Qemu-devel] [PULL] virtio-ccw: dataplane enablement

[Anthony Liguori]

Cornelia Huck <cornelia.huck@de.ibm.com> writes:

> On Tue, 16 Jul 2013 15:10:41 +0800
> Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
>> > Considering what the patch looks like, I don't think it's a huge
>> > problem...  In fact, perhaps x-data-plane could be even added to
>> > DEFINE_VIRTIO_BLK_PROPERTIES.  This would make it clear that support for
>> > non-ioeventfd hosts (including TCG) is one of the things to do to make
>> > data plane the default.
>> 
>> I agree.  Since this patch is so minimal it will not add extra work as
>> we reintegrate dataplane into core QEMU.
>> 
>> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
>> 
> Thank you.
>
> Any further guidelines on when to ask for acks from you guys?

Three days between a post and pull request is a bit short especially
with no feedback.  That was my primary concern here.

Regards,

Anthony Liguori

> The mode
> I've been operating in is mostly "if virtio-pci provides this feature,
> it shouldn't be half bad and we want it in virtio-ccw as well in order
> to keep things consistent and get more test coverage".
>
> (dataplane has been enabled in our internal testing for some time now,
> I just had asked Dominik to re-do his patch with a proper description
> as we had some code churn in our internal repository.)

Re: [Qemu-devel] [PULL] virtio-ccw: dataplane enablement

[Anthony Liguori]

Pulled.  Thanks.

Regards,

Anthony Liguori