| Message ID | 1373903207-27085-1-git-send-email-cornelia.huck@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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
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>
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.)
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
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.)
Pulled. Thanks. Regards, Anthony Liguori