[Qemu-devel] [PATCH 0/3] dataplane: introduce virtio-blk.iothread property

[Qemu-devel] [PATCH 0/3] dataplane: introduce virtio-blk.iothread property

[Stefan Hajnoczi]

Up until now the virtio-blk x-data-plane and x-iothread features have been
experimental.  It's time to commit to a final command-line interface:

  qemu -object iothread,id=iothread0 \
       -drive if=none,id=drive0,file=test.qcow2,format=qcow2 \
       -device virtio-blk-pci,iothread=iothread0,drive=drive0

The iothread=<iothread> property associates a virtio-blk-pci instance with an
IOThread.  If the iothread property is not given, the non-dataplane code path
is used.  This means dataplane is not the default yet.

The x-data-plane=on|off property is now obsolete but will be kept around for at
least another release cycle to give users a chance to migrate.

Stefan Hajnoczi (3):
  virtio-blk: replace x-iothread with iothread link property
  qdev: drop iothread property type
  virtio-blk: remove need for explicit x-data-plane=on option

 hw/block/dataplane/virtio-blk.c  |  2 +-
 hw/block/virtio-blk.c            | 12 +++++++++-
 hw/core/qdev-properties-system.c | 50 ----------------------------------------
 hw/s390x/s390-virtio-bus.c       |  2 ++
 hw/s390x/virtio-ccw.c            |  2 ++
 hw/virtio/virtio-pci.c           |  2 ++
 include/hw/qdev-properties.h     |  3 ---
 7 files changed, 18 insertions(+), 55 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/3] virtio-blk: replace x-iothread with iothread link property

[Stefan Hajnoczi]

Up until now -device virtio-blk-pci,x-iothread=<id> was used to assign
an IOThread.  This was a temporary solution while we cleaned up QOM link
properties.

This patch switches over to a QOM link property since it is now possible
to restrict the setter to unrealized instances and automatically unref
the IOThread when the virtio-blk-pci device is freed.

Since the "iothread" property is a QOM property and not a qdev property,
we must alias it explicitly for virtio-blk-pci, as well as CCW and
s390-virtio.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/virtio-blk.c      | 12 +++++++++++-
 hw/s390x/s390-virtio-bus.c |  2 ++
 hw/s390x/virtio-ccw.c      |  2 ++
 hw/virtio/virtio-pci.c     |  2 ++
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 8ac7633..03c1e91 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -761,12 +761,21 @@ static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
     virtio_cleanup(vdev);
 }
 
+static void virtio_blk_instance_init(Object *obj)
+{
+    VirtIOBlock *s = VIRTIO_BLK(obj);
+
+    object_property_add_link(obj, "iothread", TYPE_IOTHREAD,
+                             (Object **)&s->blk.iothread,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
+}
+
 static Property virtio_blk_properties[] = {
     DEFINE_BLOCK_PROPERTIES(VirtIOBlock, blk.conf),
     DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlock, blk.conf),
     DEFINE_PROP_STRING("serial", VirtIOBlock, blk.serial),
     DEFINE_PROP_BIT("config-wce", VirtIOBlock, blk.config_wce, 0, true),
-    DEFINE_PROP_IOTHREAD("x-iothread", VirtIOBlock, blk.iothread),
 #ifdef __linux__
     DEFINE_PROP_BIT("scsi", VirtIOBlock, blk.scsi, 0, true),
 #endif
@@ -796,6 +805,7 @@ static const TypeInfo virtio_device_info = {
     .name = TYPE_VIRTIO_BLK,
     .parent = TYPE_VIRTIO_DEVICE,
     .instance_size = sizeof(VirtIOBlock),
+    .instance_init = virtio_blk_instance_init,
     .class_init = virtio_blk_class_init,
 };
 
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index bf92cb6..1a171de 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -193,6 +193,8 @@ static void s390_virtio_blk_instance_init(Object *obj)
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+    object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread",
+                              &error_abort);
 }
 
 static int s390_virtio_serial_init(VirtIOS390Device *s390_dev)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 9360892..9b41671 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -815,6 +815,8 @@ static void virtio_ccw_blk_instance_init(Object *obj)
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+    object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread",
+                              &error_abort);
 }
 
 static int virtio_ccw_serial_init(VirtioCcwDevice *ccw_dev)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index abf05a9..99a66c7 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1104,6 +1104,8 @@ static void virtio_blk_pci_instance_init(Object *obj)
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+    object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread",
+                              &error_abort);
 }
 
 static const TypeInfo virtio_blk_pci_info = {
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/3] virtio-blk: replace x-iothread with iothread link property

[Paolo Bonzini]

Il 10/06/2014 09:03, Stefan Hajnoczi ha scritto:
> @@ -193,6 +193,8 @@ static void s390_virtio_blk_instance_init(Object *obj)
>      object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
>      object_unref(OBJECT(&dev->vdev));
>      qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
> +    object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread",
> +                              &error_abort);

Actually this won't work.  Link properties have to have a LinkProperty 
opaque.

The series I posted to add a "resolve" callback for properties can be 
used here, fixing object_property_add_alias to add a "resolve" callback 
to the newly-created property.

My ack for this patch still stands, it's just a matter of patch ordering.

Paolo

Re: [Qemu-devel] [PATCH 1/3] virtio-blk: replace x-iothread with iothread link property

[Stefan Hajnoczi]

On Wed, Jun 11, 2014 at 2:35 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 10/06/2014 09:03, Stefan Hajnoczi ha scritto:
>
>> @@ -193,6 +193,8 @@ static void s390_virtio_blk_instance_init(Object *obj)
>>      object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev),
>> NULL);
>>      object_unref(OBJECT(&dev->vdev));
>>      qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
>> +    object_property_add_alias(obj, "iothread",
>> OBJECT(&dev->vdev),"iothread",
>> +                              &error_abort);
>
>
> Actually this won't work.  Link properties have to have a LinkProperty
> opaque.
>
> The series I posted to add a "resolve" callback for properties can be used
> here, fixing object_property_add_alias to add a "resolve" callback to the
> newly-created property.
>
> My ack for this patch still stands, it's just a matter of patch ordering.

Okay, then it makes sense to hold this back until your patch is merged.

Stefa

Re: [Qemu-devel] [PATCH 1/3] virtio-blk: replace x-iothread with iothread link property

[Paolo Bonzini]

Il 18/06/2014 13:11, Stefan Hajnoczi ha scritto:
>> >
>> > Actually this won't work.  Link properties have to have a LinkProperty
>> > opaque.
>> >
>> > The series I posted to add a "resolve" callback for properties can be used
>> > here, fixing object_property_add_alias to add a "resolve" callback to the
>> > newly-created property.
>> >
>> > My ack for this patch still stands, it's just a matter of patch ordering.
> Okay, then it makes sense to hold this back until your patch is merged.

As discussed on IRC though, if the basic use case works, go ahead.  If 
there are bugs on fringe cases (such as iothread=vbp1/iothread where 
vbp1 is a virtio-blk-pci device), we can fix them in hard freeze.

Paolo

[Qemu-devel] [PATCH 2/3] qdev: drop iothread property type

[Stefan Hajnoczi]

The iothread property type is no longer used and can be removed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/core/qdev-properties-system.c | 50 ----------------------------------------
 include/hw/qdev-properties.h     |  3 ---
 2 files changed, 53 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 404cf18..b25b760 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -386,56 +386,6 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
     nd->instantiated = 1;
 }
 
-/* --- iothread --- */
-
-static char *print_iothread(void *ptr)
-{
-    return iothread_get_id(ptr);
-}
-
-static int parse_iothread(DeviceState *dev, const char *str, void **ptr)
-{
-    IOThread *iothread;
-
-    iothread = iothread_find(str);
-    if (!iothread) {
-        return -ENOENT;
-    }
-    object_ref(OBJECT(iothread));
-    *ptr = iothread;
-    return 0;
-}
-
-static void get_iothread(Object *obj, struct Visitor *v, void *opaque,
-                         const char *name, Error **errp)
-{
-    get_pointer(obj, v, opaque, print_iothread, name, errp);
-}
-
-static void set_iothread(Object *obj, struct Visitor *v, void *opaque,
-                         const char *name, Error **errp)
-{
-    set_pointer(obj, v, opaque, parse_iothread, name, errp);
-}
-
-static void release_iothread(Object *obj, const char *name, void *opaque)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    IOThread **ptr = qdev_get_prop_ptr(dev, prop);
-
-    if (*ptr) {
-        object_unref(OBJECT(*ptr));
-    }
-}
-
-PropertyInfo qdev_prop_iothread = {
-    .name = "iothread",
-    .get = get_iothread,
-    .set = set_iothread,
-    .release = release_iothread,
-};
-
 static int qdev_add_one_global(QemuOpts *opts, void *opaque)
 {
     GlobalProperty *g;
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index ffef425..b7ed9b9 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -22,7 +22,6 @@ extern PropertyInfo qdev_prop_bios_chs_trans;
 extern PropertyInfo qdev_prop_drive;
 extern PropertyInfo qdev_prop_netdev;
 extern PropertyInfo qdev_prop_vlan;
-extern PropertyInfo qdev_prop_iothread;
 extern PropertyInfo qdev_prop_pci_devfn;
 extern PropertyInfo qdev_prop_blocksize;
 extern PropertyInfo qdev_prop_pci_host_devaddr;
@@ -143,8 +142,6 @@ extern PropertyInfo qdev_prop_arraylen;
     DEFINE_PROP(_n, _s, _f, qdev_prop_vlan, NICPeers)
 #define DEFINE_PROP_DRIVE(_n, _s, _f) \
     DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
-#define DEFINE_PROP_IOTHREAD(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_iothread, IOThread *)
 #define DEFINE_PROP_MACADDR(_n, _s, _f)         \
     DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
 #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
-- 
1.9.3

[Qemu-devel] [PATCH 3/3] virtio-blk: remove need for explicit x-data-plane=on option

[Stefan Hajnoczi]

The x-data-plane=on|off option is no longer useful because the
iothread=<iothread> option conveys the same information plus which
IOThread to use.

Do not delete x-data-plane=on|off yet as a convenience to people using
this legacy experimental option.  We will drop it in QEMU 2.2.

Instead, turn on data-plane when either x-data-plane=on or
iothread=<iothread> are used.  The following command-line uses
data-plane:

  qemu -device virtio-blk-pci,iothread=foo,drive=drive0

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index c10b7b7..8ac6049 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -334,7 +334,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
 
     *dataplane = NULL;
 
-    if (!blk->data_plane) {
+    if (!blk->data_plane && !blk->iothread) {
         return;
     }
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH 0/3] dataplane: introduce virtio-blk.iothread property

[Paolo Bonzini]

Il 10/06/2014 09:03, Stefan Hajnoczi ha scritto:
> Up until now the virtio-blk x-data-plane and x-iothread features have been
> experimental.  It's time to commit to a final command-line interface:
>
>   qemu -object iothread,id=iothread0 \
>        -drive if=none,id=drive0,file=test.qcow2,format=qcow2 \
>        -device virtio-blk-pci,iothread=iothread0,drive=drive0
>
> The iothread=<iothread> property associates a virtio-blk-pci instance with an
> IOThread.  If the iothread property is not given, the non-dataplane code path
> is used.  This means dataplane is not the default yet.
>
> The x-data-plane=on|off property is now obsolete but will be kept around for at
> least another release cycle to give users a chance to migrate.
>
> Stefan Hajnoczi (3):
>   virtio-blk: replace x-iothread with iothread link property
>   qdev: drop iothread property type
>   virtio-blk: remove need for explicit x-data-plane=on option
>
>  hw/block/dataplane/virtio-blk.c  |  2 +-
>  hw/block/virtio-blk.c            | 12 +++++++++-
>  hw/core/qdev-properties-system.c | 50 ----------------------------------------
>  hw/s390x/s390-virtio-bus.c       |  2 ++
>  hw/s390x/virtio-ccw.c            |  2 ++
>  hw/virtio/virtio-pci.c           |  2 ++
>  include/hw/qdev-properties.h     |  3 ---
>  7 files changed, 18 insertions(+), 55 deletions(-)
>

I don't yet see object_property_add_alias, apart from that

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH 0/3] dataplane: introduce virtio-blk.iothread property

[Stefan Hajnoczi]

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

On Tue, Jun 10, 2014 at 09:03:19AM +0200, Stefan Hajnoczi wrote:
> Up until now the virtio-blk x-data-plane and x-iothread features have been
> experimental.  It's time to commit to a final command-line interface:
> 
>   qemu -object iothread,id=iothread0 \
>        -drive if=none,id=drive0,file=test.qcow2,format=qcow2 \
>        -device virtio-blk-pci,iothread=iothread0,drive=drive0
> 
> The iothread=<iothread> property associates a virtio-blk-pci instance with an
> IOThread.  If the iothread property is not given, the non-dataplane code path
> is used.  This means dataplane is not the default yet.
> 
> The x-data-plane=on|off property is now obsolete but will be kept around for at
> least another release cycle to give users a chance to migrate.
> 
> Stefan Hajnoczi (3):
>   virtio-blk: replace x-iothread with iothread link property
>   qdev: drop iothread property type
>   virtio-blk: remove need for explicit x-data-plane=on option
> 
>  hw/block/dataplane/virtio-blk.c  |  2 +-
>  hw/block/virtio-blk.c            | 12 +++++++++-
>  hw/core/qdev-properties-system.c | 50 ----------------------------------------
>  hw/s390x/s390-virtio-bus.c       |  2 ++
>  hw/s390x/virtio-ccw.c            |  2 ++
>  hw/virtio/virtio-pci.c           |  2 ++
>  include/hw/qdev-properties.h     |  3 ---
>  7 files changed, 18 insertions(+), 55 deletions(-)

Applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

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