All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option
@ 2015-12-07 10:59 Fam Zheng
  2015-12-07 11:29 ` Cornelia Huck
  0 siblings, 1 reply; 9+ messages in thread
From: Fam Zheng @ 2015-12-07 10:59 UTC (permalink / raw
  To: qemu-devel; +Cc: Kevin Wolf, pbonzini, qemu-block, Stefan Hajnoczi

The official way of enabling dataplane is through the "iothread"
property that references an iothread object created by "-object
iothread".  Since the old "x-data-plane=on" way now even crashes, it's
probably easier to just drop it:

$ qemu-system-x86_64 -drive file=null-co://,id=d0,if=none \
    -device virtio-blk-pci,drive=d0,x-data-plane=on

ERROR:/home/fam/work/qemu/qom/object.c:1515:
object_get_canonical_path_component: assertion failed: (obj->parent != NULL)
Aborted

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 15 ++-------------
 hw/block/virtio-blk.c           |  1 -
 include/hw/virtio/virtio-blk.h  |  1 -
 3 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index c42ddeb..c57f293 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -45,7 +45,6 @@ struct VirtIOBlockDataPlane {
      * use it).
      */
     IOThread *iothread;
-    IOThread internal_iothread_obj;
     AioContext *ctx;
     EventNotifier host_notifier;    /* doorbell */
 
@@ -149,14 +148,14 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
 
     *dataplane = NULL;
 
-    if (!conf->data_plane && !conf->iothread) {
+    if (!conf->iothread) {
         return;
     }
 
     /* Don't try if transport does not support notifiers. */
     if (!k->set_guest_notifiers || !k->set_host_notifier) {
         error_setg(errp,
-                   "device is incompatible with x-data-plane "
+                   "device is incompatible with dataplane "
                    "(transport does not support notifiers)");
         return;
     }
@@ -179,16 +178,6 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
     if (conf->iothread) {
         s->iothread = conf->iothread;
         object_ref(OBJECT(s->iothread));
-    } else {
-        /* Create per-device IOThread if none specified.  This is for
-         * x-data-plane option compatibility.  If x-data-plane is removed we
-         * can drop this.
-         */
-        object_initialize(&s->internal_iothread_obj,
-                          sizeof(s->internal_iothread_obj),
-                          TYPE_IOTHREAD);
-        user_creatable_complete(OBJECT(&s->internal_iothread_obj), &error_abort);
-        s->iothread = &s->internal_iothread_obj;
     }
     s->ctx = iothread_get_aio_context(s->iothread);
     s->bh = aio_bh_new(s->ctx, notify_guest_bh, s);
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 756ae5c..b88b726 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -986,7 +986,6 @@ static Property virtio_blk_properties[] = {
 #endif
     DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
                     true),
-    DEFINE_PROP_BIT("x-data-plane", VirtIOBlock, conf.data_plane, 0, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 6bf5905..ae11a63 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -37,7 +37,6 @@ struct VirtIOBlkConf
     char *serial;
     uint32_t scsi;
     uint32_t config_wce;
-    uint32_t data_plane;
     uint32_t request_merging;
 };
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option
  2015-12-07 10:59 [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option Fam Zheng
@ 2015-12-07 11:29 ` Cornelia Huck
  2015-12-07 13:02   ` Fam Zheng
  0 siblings, 1 reply; 9+ messages in thread
From: Cornelia Huck @ 2015-12-07 11:29 UTC (permalink / raw
  To: Fam Zheng; +Cc: Kevin Wolf, pbonzini, Stefan Hajnoczi, qemu-devel, qemu-block

On Mon,  7 Dec 2015 18:59:27 +0800
Fam Zheng <famz@redhat.com> wrote:

> The official way of enabling dataplane is through the "iothread"
> property that references an iothread object created by "-object
> iothread".  Since the old "x-data-plane=on" way now even crashes, it's
> probably easier to just drop it:
> 
> $ qemu-system-x86_64 -drive file=null-co://,id=d0,if=none \
>     -device virtio-blk-pci,drive=d0,x-data-plane=on
> 
> ERROR:/home/fam/work/qemu/qom/object.c:1515:
> object_get_canonical_path_component: assertion failed: (obj->parent != NULL)
> Aborted

Do we understand yet why this crashes, btw?

> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/block/dataplane/virtio-blk.c | 15 ++-------------
>  hw/block/virtio-blk.c           |  1 -
>  include/hw/virtio/virtio-blk.h  |  1 -
>  3 files changed, 2 insertions(+), 15 deletions(-)
> 

No general objection to removing x-data-plane; but this probably wants
a mention on the changelog as x-data-plane has been described in
various howtos etc. over the years.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option
  2015-12-07 11:29 ` Cornelia Huck
@ 2015-12-07 13:02   ` Fam Zheng
  2015-12-07 15:19     ` Paolo Bonzini
  2015-12-08  1:56     ` Fam Zheng
  0 siblings, 2 replies; 9+ messages in thread
From: Fam Zheng @ 2015-12-07 13:02 UTC (permalink / raw
  To: Cornelia Huck
  Cc: Kevin Wolf, pbonzini, Stefan Hajnoczi, qemu-devel, qemu-block

On Mon, 12/07 12:29, Cornelia Huck wrote:
> On Mon,  7 Dec 2015 18:59:27 +0800
> Fam Zheng <famz@redhat.com> wrote:
> 
> > The official way of enabling dataplane is through the "iothread"
> > property that references an iothread object created by "-object
> > iothread".  Since the old "x-data-plane=on" way now even crashes, it's
> > probably easier to just drop it:
> > 
> > $ qemu-system-x86_64 -drive file=null-co://,id=d0,if=none \
> >     -device virtio-blk-pci,drive=d0,x-data-plane=on
> > 
> > ERROR:/home/fam/work/qemu/qom/object.c:1515:
> > object_get_canonical_path_component: assertion failed: (obj->parent != NULL)
> > Aborted
> 
> Do we understand yet why this crashes, btw?

I think it's because with x-data-plane=on, virtio-blk initialize an object that
doesn't have a parent, therefore it doesn't have a valid "canonical path
component" thing, which is different from objects created with "-object" CLI.
I'm not very familiar with the QOM semantics here.

> 
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  hw/block/dataplane/virtio-blk.c | 15 ++-------------
> >  hw/block/virtio-blk.c           |  1 -
> >  include/hw/virtio/virtio-blk.h  |  1 -
> >  3 files changed, 2 insertions(+), 15 deletions(-)
> > 
> 
> No general objection to removing x-data-plane; but this probably wants
> a mention on the changelog as x-data-plane has been described in
> various howtos etc. over the years.
> 

Yes, that is a good point.  I don't know if it's too rushing in removing it for
2.5 (this is just posted as one option) and we'll have to count on QOM experts
for the fix, if it is.

Fam

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option
  2015-12-07 13:02   ` Fam Zheng
@ 2015-12-07 15:19     ` Paolo Bonzini
  2015-12-07 15:51       ` Cornelia Huck
  2015-12-07 17:10       ` Peter Maydell
  2015-12-08  1:56     ` Fam Zheng
  1 sibling, 2 replies; 9+ messages in thread
From: Paolo Bonzini @ 2015-12-07 15:19 UTC (permalink / raw
  To: Fam Zheng, Cornelia Huck
  Cc: Kevin Wolf, Peter Maydell, qemu-block, qemu-devel,
	Stefan Hajnoczi



On 07/12/2015 14:02, Fam Zheng wrote:
> On Mon, 12/07 12:29, Cornelia Huck wrote:
>> On Mon,  7 Dec 2015 18:59:27 +0800
>> Fam Zheng <famz@redhat.com> wrote:
>>
>>> The official way of enabling dataplane is through the "iothread"
>>> property that references an iothread object created by "-object
>>> iothread".  Since the old "x-data-plane=on" way now even crashes, it's
>>> probably easier to just drop it:
>>>
>>> $ qemu-system-x86_64 -drive file=null-co://,id=d0,if=none \
>>>     -device virtio-blk-pci,drive=d0,x-data-plane=on
>>>
>>> ERROR:/home/fam/work/qemu/qom/object.c:1515:
>>> object_get_canonical_path_component: assertion failed: (obj->parent != NULL)
>>> Aborted
>>
>> Do we understand yet why this crashes, btw?
> 
> I think it's because with x-data-plane=on, virtio-blk initialize an object that
> doesn't have a parent, therefore it doesn't have a valid "canonical path
> component" thing, which is different from objects created with "-object" CLI.
> I'm not very familiar with the QOM semantics here.
> 
>>
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> ---
>>>  hw/block/dataplane/virtio-blk.c | 15 ++-------------
>>>  hw/block/virtio-blk.c           |  1 -
>>>  include/hw/virtio/virtio-blk.h  |  1 -
>>>  3 files changed, 2 insertions(+), 15 deletions(-)
>>>
>>
>> No general objection to removing x-data-plane; but this probably wants
>> a mention on the changelog as x-data-plane has been described in
>> various howtos etc. over the years.
> 
> Yes, that is a good point.  I don't know if it's too rushing in removing it for
> 2.5 (this is just posted as one option) and we'll have to count on QOM experts
> for the fix, if it is.

The solution would be to add object_property_add_child to
virtio_blk_data_plane_create, between object_initialize and
user_creatable_complete.  But I think this patch is ok for 2.5.

Paolo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option
  2015-12-07 15:19     ` Paolo Bonzini
@ 2015-12-07 15:51       ` Cornelia Huck
  2015-12-07 17:10       ` Peter Maydell
  1 sibling, 0 replies; 9+ messages in thread
From: Cornelia Huck @ 2015-12-07 15:51 UTC (permalink / raw
  To: Paolo Bonzini
  Cc: Kevin Wolf, Peter Maydell, Fam Zheng, qemu-block, qemu-devel,
	Stefan Hajnoczi

On Mon, 7 Dec 2015 16:19:07 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> The solution would be to add object_property_add_child to
> virtio_blk_data_plane_create, between object_initialize and
> user_creatable_complete.  But I think this patch is ok for 2.5.

Just sent a patch that does this. If it is correct, I'd prefer it over
either removing x-data-plane or reverting the iothread patch.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option
  2015-12-07 15:19     ` Paolo Bonzini
  2015-12-07 15:51       ` Cornelia Huck
@ 2015-12-07 17:10       ` Peter Maydell
  2015-12-09  2:37         ` Stefan Hajnoczi
  1 sibling, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2015-12-07 17:10 UTC (permalink / raw
  To: Paolo Bonzini
  Cc: Kevin Wolf, Fam Zheng, Qemu-block, QEMU Developers,
	Stefan Hajnoczi, Cornelia Huck

On 7 December 2015 at 15:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 07/12/2015 14:02, Fam Zheng wrote:
>> On Mon, 12/07 12:29, Cornelia Huck wrote:
>>> On Mon,  7 Dec 2015 18:59:27 +0800
>>> Fam Zheng <famz@redhat.com> wrote:
>>>
>>>> The official way of enabling dataplane is through the "iothread"
>>>> property that references an iothread object created by "-object
>>>> iothread".  Since the old "x-data-plane=on" way now even crashes, it's
>>>> probably easier to just drop it:
>>>>
>>>> $ qemu-system-x86_64 -drive file=null-co://,id=d0,if=none \
>>>>     -device virtio-blk-pci,drive=d0,x-data-plane=on
>>>>
>>>> ERROR:/home/fam/work/qemu/qom/object.c:1515:
>>>> object_get_canonical_path_component: assertion failed: (obj->parent != NULL)
>>>> Aborted
>>>
>>> Do we understand yet why this crashes, btw?
>>
>> I think it's because with x-data-plane=on, virtio-blk initialize an object that
>> doesn't have a parent, therefore it doesn't have a valid "canonical path
>> component" thing, which is different from objects created with "-object" CLI.
>> I'm not very familiar with the QOM semantics here.
>>
>>>
>>>>
>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>> ---
>>>>  hw/block/dataplane/virtio-blk.c | 15 ++-------------
>>>>  hw/block/virtio-blk.c           |  1 -
>>>>  include/hw/virtio/virtio-blk.h  |  1 -
>>>>  3 files changed, 2 insertions(+), 15 deletions(-)
>>>>
>>>
>>> No general objection to removing x-data-plane; but this probably wants
>>> a mention on the changelog as x-data-plane has been described in
>>> various howtos etc. over the years.
>>
>> Yes, that is a good point.  I don't know if it's too rushing in removing it for
>> 2.5 (this is just posted as one option) and we'll have to count on QOM experts
>> for the fix, if it is.
>
> The solution would be to add object_property_add_child to
> virtio_blk_data_plane_create, between object_initialize and
> user_creatable_complete.  But I think this patch is ok for 2.5.

Paolo asked me to apply this to master, so I have done so.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option
  2015-12-07 13:02   ` Fam Zheng
  2015-12-07 15:19     ` Paolo Bonzini
@ 2015-12-08  1:56     ` Fam Zheng
  2015-12-08  9:35       ` Cornelia Huck
  1 sibling, 1 reply; 9+ messages in thread
From: Fam Zheng @ 2015-12-08  1:56 UTC (permalink / raw
  To: Cornelia Huck
  Cc: Kevin Wolf, pbonzini, qemu-block, qemu-devel, Stefan Hajnoczi

On Mon, 12/07 21:02, Fam Zheng wrote:
> On Mon, 12/07 12:29, Cornelia Huck wrote:
> > No general objection to removing x-data-plane; but this probably wants
> > a mention on the changelog as x-data-plane has been described in
> > various howtos etc. over the years.

Add a changelog line,

http://wiki.qemu.org/ChangeLog/2.5#Block_devices_and_tools

please review.

Fam

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option
  2015-12-08  1:56     ` Fam Zheng
@ 2015-12-08  9:35       ` Cornelia Huck
  0 siblings, 0 replies; 9+ messages in thread
From: Cornelia Huck @ 2015-12-08  9:35 UTC (permalink / raw
  To: Fam Zheng; +Cc: Kevin Wolf, pbonzini, qemu-block, qemu-devel, Stefan Hajnoczi

On Tue, 8 Dec 2015 09:56:14 +0800
Fam Zheng <famz@redhat.com> wrote:

> On Mon, 12/07 21:02, Fam Zheng wrote:
> > On Mon, 12/07 12:29, Cornelia Huck wrote:
> > > No general objection to removing x-data-plane; but this probably wants
> > > a mention on the changelog as x-data-plane has been described in
> > > various howtos etc. over the years.
> 
> Add a changelog line,
> 
> http://wiki.qemu.org/ChangeLog/2.5#Block_devices_and_tools
> 
> please review.

Looks sane, although I'd probably add a line to "Incompatible changes"
as well.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option
  2015-12-07 17:10       ` Peter Maydell
@ 2015-12-09  2:37         ` Stefan Hajnoczi
  0 siblings, 0 replies; 9+ messages in thread
From: Stefan Hajnoczi @ 2015-12-09  2:37 UTC (permalink / raw
  To: Peter Maydell
  Cc: Kevin Wolf, Fam Zheng, Qemu-block, QEMU Developers, Cornelia Huck,
	Paolo Bonzini

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

On Mon, Dec 07, 2015 at 05:10:26PM +0000, Peter Maydell wrote:
> On 7 December 2015 at 15:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> >
> > On 07/12/2015 14:02, Fam Zheng wrote:
> >> On Mon, 12/07 12:29, Cornelia Huck wrote:
> >>> On Mon,  7 Dec 2015 18:59:27 +0800
> >>> Fam Zheng <famz@redhat.com> wrote:
> >>>
> >>>> The official way of enabling dataplane is through the "iothread"
> >>>> property that references an iothread object created by "-object
> >>>> iothread".  Since the old "x-data-plane=on" way now even crashes, it's
> >>>> probably easier to just drop it:
> >>>>
> >>>> $ qemu-system-x86_64 -drive file=null-co://,id=d0,if=none \
> >>>>     -device virtio-blk-pci,drive=d0,x-data-plane=on
> >>>>
> >>>> ERROR:/home/fam/work/qemu/qom/object.c:1515:
> >>>> object_get_canonical_path_component: assertion failed: (obj->parent != NULL)
> >>>> Aborted
> >>>
> >>> Do we understand yet why this crashes, btw?
> >>
> >> I think it's because with x-data-plane=on, virtio-blk initialize an object that
> >> doesn't have a parent, therefore it doesn't have a valid "canonical path
> >> component" thing, which is different from objects created with "-object" CLI.
> >> I'm not very familiar with the QOM semantics here.
> >>
> >>>
> >>>>
> >>>> Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>> ---
> >>>>  hw/block/dataplane/virtio-blk.c | 15 ++-------------
> >>>>  hw/block/virtio-blk.c           |  1 -
> >>>>  include/hw/virtio/virtio-blk.h  |  1 -
> >>>>  3 files changed, 2 insertions(+), 15 deletions(-)
> >>>>
> >>>
> >>> No general objection to removing x-data-plane; but this probably wants
> >>> a mention on the changelog as x-data-plane has been described in
> >>> various howtos etc. over the years.
> >>
> >> Yes, that is a good point.  I don't know if it's too rushing in removing it for
> >> 2.5 (this is just posted as one option) and we'll have to count on QOM experts
> >> for the fix, if it is.
> >
> > The solution would be to add object_property_add_child to
> > virtio_blk_data_plane_create, between object_initialize and
> > user_creatable_complete.  But I think this patch is ok for 2.5.
> 
> Paolo asked me to apply this to master, so I have done so.

Okay.  I will do my best to communicate that x-data-plane is gone.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-12-09  2:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-07 10:59 [Qemu-devel] [PATCH] virtio-blk: Drop x-data-plane option Fam Zheng
2015-12-07 11:29 ` Cornelia Huck
2015-12-07 13:02   ` Fam Zheng
2015-12-07 15:19     ` Paolo Bonzini
2015-12-07 15:51       ` Cornelia Huck
2015-12-07 17:10       ` Peter Maydell
2015-12-09  2:37         ` Stefan Hajnoczi
2015-12-08  1:56     ` Fam Zheng
2015-12-08  9:35       ` Cornelia Huck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.