[Qemu-block] [PATCH] virtio-blk: Drop x-data-plane option
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 --- 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
Re: [Qemu-block] [PATCH] virtio-blk: Drop x-data-plane option
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 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 >>> --- >>> 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
Re: [Qemu-block] [PATCH] virtio-blk: Drop x-data-plane option
On Mon, 7 Dec 2015 16:19:07 +0100 Paolo Bonzini 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.
Re: [Qemu-block] [PATCH] virtio-blk: Drop x-data-plane option
On 7 December 2015 at 15:19, Paolo Bonzini 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 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 --- 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
Re: [Qemu-block] [PATCH] virtio-blk: Drop x-data-plane option
On Mon, Dec 07, 2015 at 05:10:26PM +, Peter Maydell wrote: > On 7 December 2015 at 15:19, Paolo Bonzini 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 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 > --- > 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 signature.asc Description: PGP signature