[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
