virtio-blk: Allow config-wce in dataplane

Dataplane now uses block layer. Protect bdrv_set_enable_write_cache with
aio_context_acquire and aio_context_release, so we can enable config-wce
to allow guest to modify the write cache online.

Signed-off-by: Fam Zheng <famz@redhat.com>

---
This applies on top of Stefan's "dataplane: use QEMU block layer"
series.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 6 ------
 hw/block/virtio-blk.c           | 9 ++++++++-
 2 files changed, 8 insertions(+), 7 deletions(-)

Il 13/05/2014 04:24, Fam Zheng ha scritto:
> Dataplane now uses block layer. Protect bdrv_set_enable_write_cache with
> aio_context_acquire and aio_context_release, so we can enable config-wce
> to allow guest to modify the write cache online.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> ---
> This applies on top of Stefan's "dataplane: use QEMU block layer"
> series.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/block/dataplane/virtio-blk.c | 6 ------
>  hw/block/virtio-blk.c           | 9 ++++++++-
>  2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index 79fb612..46a6824 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -332,12 +332,6 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
>          return;
>      }
>
> -    if (blk->config_wce) {
> -        error_setg(errp, "device is incompatible with x-data-plane, "
> -                         "use config-wce=off");
> -        return;
> -    }
> -
>      /* If dataplane is (re-)enabled while the guest is running there could be
>       * block jobs that can conflict.
>       */
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 8a568e5..a031e56 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -26,6 +26,7 @@
>  # include <scsi/sg.h>
>  #endif
>  #include "hw/virtio/virtio-bus.h"
> +#include "block/block_int.h"

You shouldn't need to include this header file.  Apart from this, the 
patch looks good.

Paolo

>
>  typedef struct VirtIOBlockReq
>  {
> @@ -523,7 +524,10 @@ static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
>      struct virtio_blk_config blkcfg;
>
>      memcpy(&blkcfg, config, sizeof(blkcfg));
> +
> +    aio_context_acquire(bdrv_get_aio_context(s->bs));
>      bdrv_set_enable_write_cache(s->bs, blkcfg.wce != 0);
> +    aio_context_release(bdrv_get_aio_context(s->bs));
>  }
>
>  static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
> @@ -582,7 +586,10 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
>       * s->bs would erroneously be placed in writethrough mode.
>       */
>      if (!(features & (1 << VIRTIO_BLK_F_CONFIG_WCE))) {
> -        bdrv_set_enable_write_cache(s->bs, !!(features & (1 << VIRTIO_BLK_F_WCE)));
> +        aio_context_acquire(bdrv_get_aio_context(s->bs));
> +        bdrv_set_enable_write_cache(s->bs,
> +                                    !!(features & (1 << VIRTIO_BLK_F_WCE)));
> +        aio_context_release(bdrv_get_aio_context(s->bs));
>      }
>  }
>
>

On Tue, 05/13 09:07, Paolo Bonzini wrote:
> Il 13/05/2014 04:24, Fam Zheng ha scritto:
> >Dataplane now uses block layer. Protect bdrv_set_enable_write_cache with
> >aio_context_acquire and aio_context_release, so we can enable config-wce
> >to allow guest to modify the write cache online.
> >
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >
> >---
> >This applies on top of Stefan's "dataplane: use QEMU block layer"
> >series.
> >
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >---
> > hw/block/dataplane/virtio-blk.c | 6 ------
> > hw/block/virtio-blk.c           | 9 ++++++++-
> > 2 files changed, 8 insertions(+), 7 deletions(-)
> >
> >diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> >index 79fb612..46a6824 100644
> >--- a/hw/block/dataplane/virtio-blk.c
> >+++ b/hw/block/dataplane/virtio-blk.c
> >@@ -332,12 +332,6 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
> >         return;
> >     }
> >
> >-    if (blk->config_wce) {
> >-        error_setg(errp, "device is incompatible with x-data-plane, "
> >-                         "use config-wce=off");
> >-        return;
> >-    }
> >-
> >     /* If dataplane is (re-)enabled while the guest is running there could be
> >      * block jobs that can conflict.
> >      */
> >diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> >index 8a568e5..a031e56 100644
> >--- a/hw/block/virtio-blk.c
> >+++ b/hw/block/virtio-blk.c
> >@@ -26,6 +26,7 @@
> > # include <scsi/sg.h>
> > #endif
> > #include "hw/virtio/virtio-bus.h"
> >+#include "block/block_int.h"
> 
> You shouldn't need to include this header file.  Apart from this, the patch
> looks good.

It's for bdrv_get_aio_context, otherwise it doesn't compile.

Fam

Il 13/05/2014 09:13, Fam Zheng ha scritto:
> On Tue, 05/13 09:07, Paolo Bonzini wrote:
>> Il 13/05/2014 04:24, Fam Zheng ha scritto:
>>> Dataplane now uses block layer. Protect bdrv_set_enable_write_cache with
>>> aio_context_acquire and aio_context_release, so we can enable config-wce
>>> to allow guest to modify the write cache online.
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>
>>> ---
>>> This applies on top of Stefan's "dataplane: use QEMU block layer"
>>> series.
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> ---
>>> hw/block/dataplane/virtio-blk.c | 6 ------
>>> hw/block/virtio-blk.c           | 9 ++++++++-
>>> 2 files changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
>>> index 79fb612..46a6824 100644
>>> --- a/hw/block/dataplane/virtio-blk.c
>>> +++ b/hw/block/dataplane/virtio-blk.c
>>> @@ -332,12 +332,6 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
>>>         return;
>>>     }
>>>
>>> -    if (blk->config_wce) {
>>> -        error_setg(errp, "device is incompatible with x-data-plane, "
>>> -                         "use config-wce=off");
>>> -        return;
>>> -    }
>>> -
>>>     /* If dataplane is (re-)enabled while the guest is running there could be
>>>      * block jobs that can conflict.
>>>      */
>>> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
>>> index 8a568e5..a031e56 100644
>>> --- a/hw/block/virtio-blk.c
>>> +++ b/hw/block/virtio-blk.c
>>> @@ -26,6 +26,7 @@
>>> # include <scsi/sg.h>
>>> #endif
>>> #include "hw/virtio/virtio-bus.h"
>>> +#include "block/block_int.h"
>>
>> You shouldn't need to include this header file.  Apart from this, the patch
>> looks good.
>
> It's for bdrv_get_aio_context, otherwise it doesn't compile.

I think it should be moved to block/block.h then.  Stefan?

Paolo

On Tue, May 13, 2014 at 11:06:51AM +0200, Paolo Bonzini wrote:
> Il 13/05/2014 09:13, Fam Zheng ha scritto:
> >On Tue, 05/13 09:07, Paolo Bonzini wrote:
> >>Il 13/05/2014 04:24, Fam Zheng ha scritto:
> >>>Dataplane now uses block layer. Protect bdrv_set_enable_write_cache with
> >>>aio_context_acquire and aio_context_release, so we can enable config-wce
> >>>to allow guest to modify the write cache online.
> >>>
> >>>Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>
> >>>---
> >>>This applies on top of Stefan's "dataplane: use QEMU block layer"
> >>>series.
> >>>
> >>>Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>---
> >>>hw/block/dataplane/virtio-blk.c | 6 ------
> >>>hw/block/virtio-blk.c           | 9 ++++++++-
> >>>2 files changed, 8 insertions(+), 7 deletions(-)
> >>>
> >>>diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> >>>index 79fb612..46a6824 100644
> >>>--- a/hw/block/dataplane/virtio-blk.c
> >>>+++ b/hw/block/dataplane/virtio-blk.c
> >>>@@ -332,12 +332,6 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
> >>>        return;
> >>>    }
> >>>
> >>>-    if (blk->config_wce) {
> >>>-        error_setg(errp, "device is incompatible with x-data-plane, "
> >>>-                         "use config-wce=off");
> >>>-        return;
> >>>-    }
> >>>-
> >>>    /* If dataplane is (re-)enabled while the guest is running there could be
> >>>     * block jobs that can conflict.
> >>>     */
> >>>diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> >>>index 8a568e5..a031e56 100644
> >>>--- a/hw/block/virtio-blk.c
> >>>+++ b/hw/block/virtio-blk.c
> >>>@@ -26,6 +26,7 @@
> >>># include <scsi/sg.h>
> >>>#endif
> >>>#include "hw/virtio/virtio-bus.h"
> >>>+#include "block/block_int.h"
> >>
> >>You shouldn't need to include this header file.  Apart from this, the patch
> >>looks good.
> >
> >It's for bdrv_get_aio_context, otherwise it doesn't compile.
> 
> I think it should be moved to block/block.h then.  Stefan?

Agreed, I was just about to reply the same thing at the top level of the
thread :).

block_int.h is for internal block layer functions that are used by the
core or block drivers.  Emulated devices must not use block_int.h (and
we'll fix files that do).

Stefan