Message ID | 1374053720-28420-4-git-send-email-stefanha@redhat.com |
---|---|
State | New |
Headers | show |
Il 17/07/2013 11:35, Stefan Hajnoczi ha scritto: > Although the dataplane thread does not cooperate with dirty memory > logging yet it's fairly easy to temporarily disable dataplane during > live migration. This way virtio-blk can live migrate when > x-data-plane=on. > > The dataplane thread will restart after migration is cancelled or if the > guest resuming virtio-blk operation after migration completes. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > hw/block/dataplane/virtio-blk.c | 17 ++++++++--------- > hw/block/virtio-blk.c | 32 ++++++++++++++++++++++++++++++++ > include/hw/virtio/virtio-blk.h | 1 + > 3 files changed, 41 insertions(+), 9 deletions(-) > > diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c > index 2faed43..411becc 100644 > --- a/hw/block/dataplane/virtio-blk.c > +++ b/hw/block/dataplane/virtio-blk.c > @@ -18,7 +18,6 @@ > #include "qemu/error-report.h" > #include "hw/virtio/dataplane/vring.h" > #include "ioq.h" > -#include "migration/migration.h" > #include "block/block.h" > #include "hw/virtio/virtio-blk.h" > #include "virtio-blk.h" > @@ -69,8 +68,6 @@ struct VirtIOBlockDataPlane { > queue */ > > unsigned int num_reqs; > - > - Error *migration_blocker; > }; > > /* Raise an interrupt to signal guest, if necessary */ > @@ -418,6 +415,14 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, > return false; > } > > + /* If dataplane is (re-)enabled while the guest is running there could be > + * block jobs that can conflict. > + */ > + if (bdrv_in_use(blk->conf.bs)) { > + error_report("cannot start dataplane thread while device is in use"); > + return false; > + } > + > fd = raw_get_aio_fd(blk->conf.bs); > if (fd < 0) { > error_report("drive is incompatible with x-data-plane, " > @@ -433,10 +438,6 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, > /* Prevent block operations that conflict with data plane thread */ > bdrv_set_in_use(blk->conf.bs, 1); > > - error_setg(&s->migration_blocker, > - "x-data-plane does not support migration"); > - migrate_add_blocker(s->migration_blocker); > - > *dataplane = s; > return true; > } > @@ -448,8 +449,6 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s) > } > > virtio_blk_data_plane_stop(s); > - migrate_del_blocker(s->migration_blocker); > - error_free(s->migration_blocker); > bdrv_set_in_use(s->blk->conf.bs, 0); > g_free(s); > } > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index cf12469..cca0c77 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -19,6 +19,7 @@ > #include "hw/virtio/virtio-blk.h" > #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE > # include "dataplane/virtio-blk.h" > +# include "migration/migration.h" > #endif > #include "block/scsi.h" > #ifdef __linux__ > @@ -628,6 +629,34 @@ void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk) > memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf)); > } > > +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE > +/* Disable dataplane thread during live migration since it does not > + * update the dirty memory bitmap yet. > + */ > +static void virtio_blk_migration_state_changed(Notifier *notifier, void *data) > +{ > + VirtIOBlock *s = container_of(notifier, VirtIOBlock, > + migration_state_notifier); > + MigrationState *mig = data; > + > + if (migration_is_active(mig)) { > + if (!s->dataplane) { > + return; > + } > + virtio_blk_data_plane_destroy(s->dataplane); > + s->dataplane = NULL; > + } else if (migration_has_finished(mig) || > + migration_has_failed(mig)) { > + if (s->dataplane) { > + return; > + } > + bdrv_drain_all(); /* complete in-flight non-dataplane requests */ > + virtio_blk_data_plane_create(VIRTIO_DEVICE(s), &s->blk, > + &s->dataplane); > + } > +} Perhaps you can call bdrv_set_in_use here (set it to 1 after destruction, and to 0 before creation), so that you do not need the check in virtio_blk_data_plane_create? > +#endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */ > + > static int virtio_blk_device_init(VirtIODevice *vdev) > { > DeviceState *qdev = DEVICE(vdev); > @@ -664,6 +693,8 @@ static int virtio_blk_device_init(VirtIODevice *vdev) > virtio_cleanup(vdev); > return -1; > } > + s->migration_state_notifier.notify = virtio_blk_migration_state_changed; > + add_migration_state_change_notifier(&s->migration_state_notifier); > #endif > > s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s); > @@ -683,6 +714,7 @@ static int virtio_blk_device_exit(DeviceState *dev) > VirtIODevice *vdev = VIRTIO_DEVICE(dev); > VirtIOBlock *s = VIRTIO_BLK(dev); > #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE > + remove_migration_state_change_notifier(&s->migration_state_notifier); > virtio_blk_data_plane_destroy(s->dataplane); > s->dataplane = NULL; > #endif > diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h > index fc71853..b87cf49 100644 > --- a/include/hw/virtio/virtio-blk.h > +++ b/include/hw/virtio/virtio-blk.h > @@ -125,6 +125,7 @@ typedef struct VirtIOBlock { > unsigned short sector_mask; > VMChangeStateEntry *change; > #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE > + Notifier migration_state_notifier; > struct VirtIOBlockDataPlane *dataplane; > #endif > } VirtIOBlock; > Only a stopgap measure, but it's self-contained and easy to revert, which makes it a brilliant solution. Just one nit above to make it even more self-contained. Paolo
On Wed, Jul 17, 2013 at 12:26:54PM +0200, Paolo Bonzini wrote: > Il 17/07/2013 11:35, Stefan Hajnoczi ha scritto: > > @@ -628,6 +629,34 @@ void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk) > > memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf)); > > } > > > > +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE > > +/* Disable dataplane thread during live migration since it does not > > + * update the dirty memory bitmap yet. > > + */ > > +static void virtio_blk_migration_state_changed(Notifier *notifier, void *data) > > +{ > > + VirtIOBlock *s = container_of(notifier, VirtIOBlock, > > + migration_state_notifier); > > + MigrationState *mig = data; > > + > > + if (migration_is_active(mig)) { > > + if (!s->dataplane) { > > + return; > > + } > > + virtio_blk_data_plane_destroy(s->dataplane); > > + s->dataplane = NULL; > > + } else if (migration_has_finished(mig) || > > + migration_has_failed(mig)) { > > + if (s->dataplane) { > > + return; > > + } > > + bdrv_drain_all(); /* complete in-flight non-dataplane requests */ > > + virtio_blk_data_plane_create(VIRTIO_DEVICE(s), &s->blk, > > + &s->dataplane); > > + } > > +} > > Perhaps you can call bdrv_set_in_use here (set it to 1 after > destruction, and to 0 before creation), so that you do not need the > check in virtio_blk_data_plane_create? The bdrv_in_use() check fixes a bug that was present before this series. Therefore I split it into a separate commit in v3 and CCed qemu-stable@nongnu.org. Stefan
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 2faed43..411becc 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -18,7 +18,6 @@ #include "qemu/error-report.h" #include "hw/virtio/dataplane/vring.h" #include "ioq.h" -#include "migration/migration.h" #include "block/block.h" #include "hw/virtio/virtio-blk.h" #include "virtio-blk.h" @@ -69,8 +68,6 @@ struct VirtIOBlockDataPlane { queue */ unsigned int num_reqs; - - Error *migration_blocker; }; /* Raise an interrupt to signal guest, if necessary */ @@ -418,6 +415,14 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, return false; } + /* If dataplane is (re-)enabled while the guest is running there could be + * block jobs that can conflict. + */ + if (bdrv_in_use(blk->conf.bs)) { + error_report("cannot start dataplane thread while device is in use"); + return false; + } + fd = raw_get_aio_fd(blk->conf.bs); if (fd < 0) { error_report("drive is incompatible with x-data-plane, " @@ -433,10 +438,6 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, /* Prevent block operations that conflict with data plane thread */ bdrv_set_in_use(blk->conf.bs, 1); - error_setg(&s->migration_blocker, - "x-data-plane does not support migration"); - migrate_add_blocker(s->migration_blocker); - *dataplane = s; return true; } @@ -448,8 +449,6 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s) } virtio_blk_data_plane_stop(s); - migrate_del_blocker(s->migration_blocker); - error_free(s->migration_blocker); bdrv_set_in_use(s->blk->conf.bs, 0); g_free(s); } diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index cf12469..cca0c77 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -19,6 +19,7 @@ #include "hw/virtio/virtio-blk.h" #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE # include "dataplane/virtio-blk.h" +# include "migration/migration.h" #endif #include "block/scsi.h" #ifdef __linux__ @@ -628,6 +629,34 @@ void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk) memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf)); } +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE +/* Disable dataplane thread during live migration since it does not + * update the dirty memory bitmap yet. + */ +static void virtio_blk_migration_state_changed(Notifier *notifier, void *data) +{ + VirtIOBlock *s = container_of(notifier, VirtIOBlock, + migration_state_notifier); + MigrationState *mig = data; + + if (migration_is_active(mig)) { + if (!s->dataplane) { + return; + } + virtio_blk_data_plane_destroy(s->dataplane); + s->dataplane = NULL; + } else if (migration_has_finished(mig) || + migration_has_failed(mig)) { + if (s->dataplane) { + return; + } + bdrv_drain_all(); /* complete in-flight non-dataplane requests */ + virtio_blk_data_plane_create(VIRTIO_DEVICE(s), &s->blk, + &s->dataplane); + } +} +#endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */ + static int virtio_blk_device_init(VirtIODevice *vdev) { DeviceState *qdev = DEVICE(vdev); @@ -664,6 +693,8 @@ static int virtio_blk_device_init(VirtIODevice *vdev) virtio_cleanup(vdev); return -1; } + s->migration_state_notifier.notify = virtio_blk_migration_state_changed; + add_migration_state_change_notifier(&s->migration_state_notifier); #endif s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s); @@ -683,6 +714,7 @@ static int virtio_blk_device_exit(DeviceState *dev) VirtIODevice *vdev = VIRTIO_DEVICE(dev); VirtIOBlock *s = VIRTIO_BLK(dev); #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE + remove_migration_state_change_notifier(&s->migration_state_notifier); virtio_blk_data_plane_destroy(s->dataplane); s->dataplane = NULL; #endif diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index fc71853..b87cf49 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -125,6 +125,7 @@ typedef struct VirtIOBlock { unsigned short sector_mask; VMChangeStateEntry *change; #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE + Notifier migration_state_notifier; struct VirtIOBlockDataPlane *dataplane; #endif } VirtIOBlock;
Although the dataplane thread does not cooperate with dirty memory logging yet it's fairly easy to temporarily disable dataplane during live migration. This way virtio-blk can live migrate when x-data-plane=on. The dataplane thread will restart after migration is cancelled or if the guest resuming virtio-blk operation after migration completes. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/block/dataplane/virtio-blk.c | 17 ++++++++--------- hw/block/virtio-blk.c | 32 ++++++++++++++++++++++++++++++++ include/hw/virtio/virtio-blk.h | 1 + 3 files changed, 41 insertions(+), 9 deletions(-)